You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/09/02 16:13:28 UTC

[GitHub] [incubator-nuttx] vbenso opened a new pull request, #6992: WS2812 LED driver using ESP32's RMT peripheral

vbenso opened a new pull request, #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992

   ## Summary
   This PR adds support for using the ESP32's RMT peripheral in transmission only mode.
   With this PR the ws2812 led driver can use ESP32's RMT peripheral to transmit data to the LEDs. Allowing an alternative to the previous method that was using SPI only.
   
   ## Impact
   The implementation besides adding files for the definition of the registers and communication with the peripheral, also adds some code to the esp32-devkitc.h bring_up function, following the pattern used with the already existing peripherals.
   
   ## Testing
   The ws2812 application from the apps repository can be used for testing.
   So far I've tested with up to 768 leds and there are no glitches.
   
   ## Further notes
   Perhaps another layer of abstraction can be built someday in order to provide a framebuffer interface for driver.
   [https://youtu.be/rmOh9iGzwtk](url)
   


-- 
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

[GitHub] [nuttx] acassis commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1605730856

   Hi @vbenso nice work! You need to rebase your branch with current upstream. Did you follow the documentation explaining how to create the upstream branch to refer to NuttX mainline: https://nuttx.apache.org/docs/latest/contributing/making-changes.html# ?


-- 
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

[GitHub] [nuttx] vbenso commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "vbenso (via GitHub)" <gi...@apache.org>.
vbenso commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1642776949

   Hi @acassis @cederom 
   Is this ok?


-- 
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

[GitHub] [incubator-nuttx] acassis commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
acassis commented on PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#issuecomment-1235724901

   @vbenso please fix the issues reported by CI compilation.
   Suggestion: since it is RMT specific for WS8212 maybe it is better to rename esp32_rmt.c to esp32_rmt_ws8212.c or similar


-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961992802


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);

Review Comment:
   > Isn't it a bit slower then getreg32+putreg32?
   
   Yes, but the performance penalty is negligible.
   When not in SMP mode, it is just a write to (read from) a special register.
   Under SMP mode it indeed spins in a lock, but I wouldn't worry about this micro-optimization if not really required.



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961935421


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);

Review Comment:
   When setting any value that not **2** I get this:
   
   
   ``
   [    0.040000] [CPU1] up_assert: Assertion failed at file:chip/esp32_irq.c line: 607 task: nsh_main
   [    0.040000] [CPU1] xtensa_dumpstate: CPU1:
   ``
   Then after some seconds, comes the rest of the stack dump.
   I'm really not confortable with using 2 because it is the only value that will actually work. Have you ever encountered such thing when dealing with interrupt priorities?



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961985563


##########
boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c:
##########
@@ -0,0 +1,333 @@
+/****************************************************************************
+ * boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include "xtensa.h"
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/spi/spi.h>
+#include <nuttx/leds/ws2812.h>
+#include "esp32_rmt.h"
+
+//#include "stm32.h"
+//#include "stm32_spi.h"
+
+#ifdef CONFIG_WS2812
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#  ifndef CONFIG_WS2812_NON_SPI_DRIVER
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver.
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/leddrvN
+ *   spino - SPI port number
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+int board_ws2812_initialize(int devno, int spino, uint16_t nleds)
+{
+  struct spi_dev_s *spi;
+  char devpath[13];
+  int ret;
+
+  spi = esp32_spibus_initialize(spino);
+  if (spi == NULL)
+  {
+    return -ENODEV;
+  }
+
+  /* Register the WS2812 driver at the specified location. */
+
+  snprintf(devpath, 13, "/dev/leds%d", devno);
+  ret = ws2812_leds_register(devpath, spi, nleds);
+  if (ret < 0)
+  {
+    lederr("ERROR: ws2812_leds_register(%s) failed: %d\n",
+           devpath, ret);
+    return ret;
+  }
+
+  return OK;
+}
+#  else
+
+struct ws2812_rmt_s
+{
+  struct rmt_dev_s *rmt_dev;
+  size_t open_count;   /* Number of opens on this instance. */
+};
+
+static int ws2812_rmt_open(FAR struct file *filep)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  int ret;
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+  if (priv->open_count == 0)
+  {
+    ledinfo("rmt_dev=%p\n", priv->rmt_dev);
+    if (priv->rmt_dev)
+    {
+
+      uint32_t reg0_addr = RMT_CHnCONF0_REG(priv->rmt_dev->channel);
+      uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+      uint32_t reg_val = 0x00;
+
+      /* a single memory block with double buffering is enough */
+      uint32_t mem_blocks = 1; 
+      priv->rmt_dev->available_words = 64*mem_blocks;
+
+      uint32_t start_addr_chn = 0x3FF56800 + 64 * 4 * priv->rmt_dev->channel;
+      priv->rmt_dev->start_address = start_addr_chn;
+      
+
+      priv->rmt_dev->reload_thresh = priv->rmt_dev->available_words/2;
+
+      reg_val = (mem_blocks) << 24;
+      uint32_t clock_divider = 2;
+      reg_val |= (clock_divider);
+      putreg32(reg_val, reg0_addr);
+
+      reg_val = 0;
+      /* use APB clock */
+      reg_val |= RMT_REF_ALWAYS_ON_CHn; 
+      /* memory block in transmission mode */
+      reg_val &= ~RMT_MEM_OWNER_CHn;    
+      putreg32(reg_val, reg1_addr);
+
+      /* set when the buffer swapping IRQ must be generated */
+      uint32_t reload_addr = RMT_CHn_TX_LIM_REG(priv->rmt_dev->channel);
+      putreg32(priv->rmt_dev->reload_thresh, reload_addr);
+      
+      /* allow direct access to RMT's memory */
+      modifyreg32(RMT_APB_CONF_REG, 0, BIT(0)); 
+    }
+  }
+
+  priv->open_count += 1;
+
+  ret = OK;
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+static ssize_t ws2812_rmt_write(FAR struct file *filep,
+                        FAR const char *data,
+                        size_t len)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  if (data == NULL)
+  {
+    lederr("DATA is NULL\n");
+    return 0;
+  }
+  
+  nxsem_wait(&dev_data->exclsem);
+
+  uint32_t mem = priv->rmt_dev->start_address;
+  uint32_t used_words = 0;
+  uint32_t current_pixel = 0;
+  uint8_t *pixel_ptr = (uint8_t *)data;
+  uint8_t pixel;
+  uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+  modifyreg32(reg1_addr, 0, RMT_MEM_RD_RST_CHn);
+  modifyreg32(reg1_addr, RMT_MEM_RD_RST_CHn, 0);
+
+  while(used_words<priv->rmt_dev->available_words)
+    {
+      if (current_pixel < len)
+        {
+          pixel = *pixel_ptr++;
+          for (int i = 0; i < 8; i++)
+            {
+              int bit = pixel & 0x80L;
+              if (bit)
+                {
+                  putreg32(LOGIC_ONE, mem);
+                }
+              else
+               {
+                 putreg32(LOGIC_ZERO, mem);
+               }
+              mem += 4;
+              used_words++;
+              pixel <<= 1;
+            }
+          current_pixel++;
+        } 
+      else 
+        {
+          putreg32(0, mem); 
+          break;
+        }
+    }
+  
+  if(used_words<priv->rmt_dev->available_words) 
+    {
+      /* Small transactions won't need a the swap buffer IRQ */
+      modifyreg32(
+        RMT_INT_ENA_REG, 
+        RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel), 
+        RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel));
+    } 
+  else 
+    {
+    /* In large transactions inform the current state to the IRQ */
+    priv->rmt_dev->next_pixel_to_load = current_pixel;
+    priv->rmt_dev->data_ptr = (uint8_t*) data;
+    priv->rmt_dev->data_len = len;
+    priv->rmt_dev->frame = 0;
+    
+    /* Enable memory wrap around */
+    modifyreg32(RMT_APB_CONF_REG, 0, BIT(1));    
+
+    /* Enable buffering IRQ */
+    modifyreg32(
+      RMT_INT_ENA_REG, 
+      0, 
+      RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel) | 
+      RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel));
+  }
+
+  /* Start the transaction */
+  modifyreg32(reg1_addr, 0, RMT_TX_START_CHn);
+  nxsem_wait(&priv->rmt_dev->tx_sem);
+
+  /* Wait for the TX lock and release it after */
+  nxsem_wait(&priv->rmt_dev->tx_sem); 
+  nxsem_post(&priv->rmt_dev->tx_sem);
+  nxsem_post(&dev_data->exclsem);
+
+  return len;
+}
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver using the RMT 
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/ledsN
+ *   rmt_dev - Pointer to the RMT device that will be used
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int board_ws2812_initialize(int devno, uint16_t nleds, struct rmt_dev_s *rmt_dev)
+{
+  char devpath[13];
+  int ret;
+
+  FAR struct ws2812_dev_s *dev_data;

Review Comment:
   Yes. `FAR` and `CODE` have been wiped from every arch implementation some time ago, since they have an empty definition for newer archs and just contribute to code bloat.



-- 
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

[GitHub] [nuttx] cederom commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1589723713

   Thank you @vbenso for the feedback and Your works :-)
   * I am working on a project right now based on ESP32 that will use WS2812 and I have custom hardware ready for testing the code (did not make it with current drivers).
   * I am working on other drivers right now, but after these are done I will get into the WS2812 part.
   * Some time ago I had ESP32+WS2812 working in MicroPython using both bitbang and RMT driver. RMT seems better, but I there was no option to use inverted signals to drive WS2812 over single MOSFET. It would be nice to have in NuttX :-)


-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961872141


##########
boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c:
##########
@@ -0,0 +1,333 @@
+/****************************************************************************
+ * boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include "xtensa.h"
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/spi/spi.h>
+#include <nuttx/leds/ws2812.h>
+#include "esp32_rmt.h"
+
+//#include "stm32.h"
+//#include "stm32_spi.h"
+
+#ifdef CONFIG_WS2812
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#  ifndef CONFIG_WS2812_NON_SPI_DRIVER
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver.
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/leddrvN
+ *   spino - SPI port number
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+int board_ws2812_initialize(int devno, int spino, uint16_t nleds)
+{
+  struct spi_dev_s *spi;
+  char devpath[13];
+  int ret;
+
+  spi = esp32_spibus_initialize(spino);
+  if (spi == NULL)
+  {
+    return -ENODEV;
+  }
+
+  /* Register the WS2812 driver at the specified location. */
+
+  snprintf(devpath, 13, "/dev/leds%d", devno);
+  ret = ws2812_leds_register(devpath, spi, nleds);
+  if (ret < 0)
+  {
+    lederr("ERROR: ws2812_leds_register(%s) failed: %d\n",
+           devpath, ret);
+    return ret;
+  }
+
+  return OK;
+}
+#  else
+
+struct ws2812_rmt_s
+{
+  struct rmt_dev_s *rmt_dev;
+  size_t open_count;   /* Number of opens on this instance. */
+};
+
+static int ws2812_rmt_open(FAR struct file *filep)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  int ret;
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+  if (priv->open_count == 0)
+  {
+    ledinfo("rmt_dev=%p\n", priv->rmt_dev);
+    if (priv->rmt_dev)
+    {
+
+      uint32_t reg0_addr = RMT_CHnCONF0_REG(priv->rmt_dev->channel);
+      uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+      uint32_t reg_val = 0x00;
+
+      /* a single memory block with double buffering is enough */
+      uint32_t mem_blocks = 1; 
+      priv->rmt_dev->available_words = 64*mem_blocks;
+
+      uint32_t start_addr_chn = 0x3FF56800 + 64 * 4 * priv->rmt_dev->channel;
+      priv->rmt_dev->start_address = start_addr_chn;
+      
+
+      priv->rmt_dev->reload_thresh = priv->rmt_dev->available_words/2;
+
+      reg_val = (mem_blocks) << 24;
+      uint32_t clock_divider = 2;
+      reg_val |= (clock_divider);
+      putreg32(reg_val, reg0_addr);
+
+      reg_val = 0;
+      /* use APB clock */
+      reg_val |= RMT_REF_ALWAYS_ON_CHn; 
+      /* memory block in transmission mode */
+      reg_val &= ~RMT_MEM_OWNER_CHn;    
+      putreg32(reg_val, reg1_addr);
+
+      /* set when the buffer swapping IRQ must be generated */
+      uint32_t reload_addr = RMT_CHn_TX_LIM_REG(priv->rmt_dev->channel);
+      putreg32(priv->rmt_dev->reload_thresh, reload_addr);
+      
+      /* allow direct access to RMT's memory */
+      modifyreg32(RMT_APB_CONF_REG, 0, BIT(0)); 
+    }
+  }
+
+  priv->open_count += 1;
+
+  ret = OK;
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+static ssize_t ws2812_rmt_write(FAR struct file *filep,
+                        FAR const char *data,
+                        size_t len)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  if (data == NULL)
+  {
+    lederr("DATA is NULL\n");
+    return 0;
+  }
+  
+  nxsem_wait(&dev_data->exclsem);
+
+  uint32_t mem = priv->rmt_dev->start_address;
+  uint32_t used_words = 0;
+  uint32_t current_pixel = 0;
+  uint8_t *pixel_ptr = (uint8_t *)data;
+  uint8_t pixel;
+  uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+  modifyreg32(reg1_addr, 0, RMT_MEM_RD_RST_CHn);
+  modifyreg32(reg1_addr, RMT_MEM_RD_RST_CHn, 0);
+
+  while(used_words<priv->rmt_dev->available_words)
+    {
+      if (current_pixel < len)
+        {
+          pixel = *pixel_ptr++;
+          for (int i = 0; i < 8; i++)
+            {
+              int bit = pixel & 0x80L;
+              if (bit)
+                {
+                  putreg32(LOGIC_ONE, mem);
+                }
+              else
+               {
+                 putreg32(LOGIC_ZERO, mem);
+               }
+              mem += 4;
+              used_words++;
+              pixel <<= 1;
+            }
+          current_pixel++;
+        } 
+      else 
+        {
+          putreg32(0, mem); 
+          break;
+        }
+    }
+  
+  if(used_words<priv->rmt_dev->available_words) 
+    {
+      /* Small transactions won't need a the swap buffer IRQ */
+      modifyreg32(
+        RMT_INT_ENA_REG, 
+        RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel), 
+        RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel));
+    } 
+  else 
+    {
+    /* In large transactions inform the current state to the IRQ */
+    priv->rmt_dev->next_pixel_to_load = current_pixel;
+    priv->rmt_dev->data_ptr = (uint8_t*) data;
+    priv->rmt_dev->data_len = len;
+    priv->rmt_dev->frame = 0;
+    
+    /* Enable memory wrap around */
+    modifyreg32(RMT_APB_CONF_REG, 0, BIT(1));    
+
+    /* Enable buffering IRQ */
+    modifyreg32(
+      RMT_INT_ENA_REG, 
+      0, 
+      RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel) | 
+      RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel));
+  }
+
+  /* Start the transaction */
+  modifyreg32(reg1_addr, 0, RMT_TX_START_CHn);
+  nxsem_wait(&priv->rmt_dev->tx_sem);
+
+  /* Wait for the TX lock and release it after */
+  nxsem_wait(&priv->rmt_dev->tx_sem); 
+  nxsem_post(&priv->rmt_dev->tx_sem);
+  nxsem_post(&dev_data->exclsem);
+
+  return len;
+}
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver using the RMT 
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/ledsN
+ *   rmt_dev - Pointer to the RMT device that will be used
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int board_ws2812_initialize(int devno, uint16_t nleds, struct rmt_dev_s *rmt_dev)
+{
+  char devpath[13];
+  int ret;
+
+  FAR struct ws2812_dev_s *dev_data;

Review Comment:
   ```suggestion
     struct ws2812_dev_s *dev_data;
   ```
   `FAR` qualifier should not be used in arch-specific code.
   Please remove other occurrences as well.



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961940471


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);

Review Comment:
   See the comment below: https://github.com/apache/incubator-nuttx/pull/6992/files/d7fd5aaececa41f1c14625701c2871b9cca4cbd3#r961940027
   
   That might be the reason it is crashing.



-- 
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

[GitHub] [incubator-nuttx] acassis commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961908760


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;

Review Comment:
   ```suggestion:
         if(dev->frame++ == 1)
           {
             dst_mem += dev->reload_thresh * 4;
           }



##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;
+      
+      if (dev->frame > 1)
+        dev->frame = 0;

Review Comment:
   ```suggestion:
         if (dev->frame > 1)
           {
              dev->frame = 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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961887060


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);

Review Comment:
   ```suggestion
   static void rmt_reset(struct rmt_dev_s *dev);
   
   IRAM_ATTR static int rmt_interrupt(int irq, void *context, void *arg);
   ```
   Since these functions are private thus restricted to ESP32, the `esp32_`  prefix may be omitted.



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961992521


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);

Review Comment:
   My mistake, 
   I was thinking that 1 was the highest priority. I'll change it to 1.
   About the twai, I'll submit the PR later today.



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961873062


##########
boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c:
##########
@@ -0,0 +1,333 @@
+/****************************************************************************
+ * boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include "xtensa.h"
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/spi/spi.h>
+#include <nuttx/leds/ws2812.h>
+#include "esp32_rmt.h"
+
+//#include "stm32.h"
+//#include "stm32_spi.h"

Review Comment:
   ```suggestion
   ```



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961940027


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);

Review Comment:
   ```suggestion
     up_enable_irq(dev->irq);
   ```
   Here `up_enable_irq` expects the IRQ number, not the internal CPU interrupt index.



-- 
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

[GitHub] [nuttx] cederom commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1648817956

   Hello world :-) I have created a dedicated Issue https://github.com/apache/nuttx/issues/9885 as there may be more optimization PRs required :-)


-- 
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

[GitHub] [nuttx] cederom commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1553624904

   Hey there @vbenso :-) Did you manage do implement the updates? How can I help? I have ESP32 + WS2812 setup ready for testing / help in development :-)
   
   One question here: would it be possible to generate inverted pulses somehow with RMT so WS2812 gets signals over N-MOSFET transistor? It will invert signals but can interface 3.3V to 5V in case of devices that must have 5V DIO and does not accept direct 3.3V (like WS2812-MINI).


-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961879513


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;
+      
+      if (dev->frame > 1)
+        dev->frame = 0;
+      
+      used_words = 0;
+      pixel_ptr = dev->data_ptr + current_pixel;
+  
+      while (used_words < dev->reload_thresh)
+        {
+          if (current_pixel < total_pixels)
+            {
+              register uint8_t pixel = (*pixel_ptr++);
+              for (register uint32_t i = 0; i < 8; i++)
+                {
+                  if (pixel & 0x80L)
+                    {
+                      putreg32(LOGIC_ONE, dst_mem);
+                    }
+                  else
+                    {
+                      putreg32(LOGIC_ZERO, dst_mem);
+                    }
+                  pixel <<= 1;
+                  dst_mem += 4;
+                }
+              used_words+=8;
+              current_pixel++;
+            }
+          else
+            {
+              regval = getreg32(RMT_INT_ENA_REG);
+              regval |= RMT_CHn_TX_END_INT_ENA(dev->channel);
+              putreg32(regval, RMT_INT_ENA_REG);
+              putreg32(0, dst_mem);
+              break;
+            }
+        }
+      
+      dev->next_pixel_to_load = current_pixel;
+    }
+  else
+    {
+      /* Perhaps an error, have no info on that */
+      putreg32(regval, RMT_INT_CLR_REG);
+    }
+  
+  return OK;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32_rmtinitialize
+ *
+ * Description:
+ *   Initialize the selected RMT device
+ *
+ * Input Parameters:
+ *   output_pin - the pin used for output
+ *   channel    - the RMT's channel that will be used
+ *
+ * Returned Value:
+ *   Valid RMT device structure reference on success; a NULL on failure
+ *
+ ****************************************************************************/
+
+struct rmt_dev_s *esp32_rmtinitialize(int output_pin, int channel)
+{
+  struct rmt_dev_s *rmtdev;
+  irqstate_t flags;
+
+  flags = enter_critical_section();

Review Comment:
   For ensuring correctness of this driver when SMP is enabled, we should use the `spin_lock_[irqsave]/[irqrestore]` APIs with a device-specific lock.
   When SMP is enabled,  `enter_critical_section` disables the interrupts only in the currently executing core, thus it does not prevent the other cores from accessing the critical section.
   
   Please refer to other peripheral drivers from ESP32 (e.g. SPI).



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961904201


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;
+      
+      if (dev->frame > 1)
+        dev->frame = 0;
+      
+      used_words = 0;
+      pixel_ptr = dev->data_ptr + current_pixel;
+  
+      while (used_words < dev->reload_thresh)
+        {
+          if (current_pixel < total_pixels)
+            {
+              register uint8_t pixel = (*pixel_ptr++);
+              for (register uint32_t i = 0; i < 8; i++)
+                {
+                  if (pixel & 0x80L)
+                    {
+                      putreg32(LOGIC_ONE, dst_mem);
+                    }
+                  else
+                    {
+                      putreg32(LOGIC_ZERO, dst_mem);
+                    }
+                  pixel <<= 1;
+                  dst_mem += 4;
+                }
+              used_words+=8;
+              current_pixel++;
+            }
+          else
+            {
+              regval = getreg32(RMT_INT_ENA_REG);
+              regval |= RMT_CHn_TX_END_INT_ENA(dev->channel);
+              putreg32(regval, RMT_INT_ENA_REG);
+              putreg32(0, dst_mem);
+              break;
+            }
+        }
+      
+      dev->next_pixel_to_load = current_pixel;
+    }

Review Comment:
   If that is the case, this application-specific logic should be called via a callback, similarly to how an application is notified of a Timer expiration.
   https://nuttx.apache.org/docs/latest/components/drivers/character/timer.html#c.TCIOC_NOTIFICATION
   



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961968869


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);

Review Comment:
   Thanks, 
   That was indeed the reason of the crash, however with any priority above 2, the IRQ isn't called on time to re-fill the RMT's buffer, and the ws2812 won't work. Perhaps for another application that uses larger pulses, there will be enough time to re-fill the buffer when using 3, 4 or 5.
   
   In a side note, esp32twai_setup is calling:
   ``
   up_enable_irq(priv->cpuint);
   ``



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#issuecomment-1235883117

   Hi Everyone,
   Thanks for all the comments, I've learned so much.
   All the style issues are already checked with checkpatch.sh.
   The function's prototypes have been moved to the proper places.
   The PIN and CHANNEL defines have been removed from the Kconfig to esp32-devkitc.h.
   There is still the issue with using or not modifyreg32 inside the interrupt function, will it impact in the performance somehow?
   Also, the values of LOGIC_ONE and LOGIC_ZERO will be moved from the RMT layer to the WS2812. 
   
   Once I finish this remaining tasks, I'll re-submit the PR.
   Thanks again for all the help, sorry for taking so much of your time.


-- 
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

[GitHub] [nuttx] acassis merged pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis merged PR #6992:
URL: https://github.com/apache/nuttx/pull/6992


-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961870245


##########
drivers/leds/Kconfig:
##########
@@ -123,6 +123,22 @@ config WS2812_NON_SPI_DRIVER
 		implementations.  Selecting this option builds the new
 		model driver.
 
+config WS2812_RMT_OUTPUT_PIN
+	int "The pin where the ws2812 are connected."
+	depends on WS2812_NON_SPI_DRIVER
+	default 4
+	range 1 35
+	---help---
+		The GPIO pin where the ws2812 are connected.

Review Comment:
   It is not appropriate for mapping GPIO pins via Kconfig, which is a board characteristic that should be immutable.
   As an example, see how the LCD display of the [ESP-WROVER-KIT board](https://nuttx.apache.org/docs/latest/platforms/xtensa/esp32/boards/esp32-wrover-kit/index.html) is mapped on its interface: https://github.com/apache/incubator-nuttx/blob/c5785ee9d5cf6d6ee7626316361a4735a0ee5195/boards/xtensa/esp32/esp32-wrover-kit/include/board.h#L42-L47



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961871444


##########
drivers/leds/Kconfig:
##########
@@ -123,6 +123,22 @@ config WS2812_NON_SPI_DRIVER
 		implementations.  Selecting this option builds the new
 		model driver.
 
+config WS2812_RMT_OUTPUT_PIN
+	int "The pin where the ws2812 are connected."
+	depends on WS2812_NON_SPI_DRIVER
+	default 4
+	range 1 35
+	---help---
+		The GPIO pin where the ws2812 are connected.
+
+config WS2812_RMT_CHANNEL
+	int "The RMT's channel used to drive the ws2812"
+	depends on WS2812_NON_SPI_DRIVER
+	default 0
+	range 0 7
+	---help---
+		Which of the RMT's channels that will be used to drive de LEDs.
+

Review Comment:
   As already commented by @xiaoxiang781216, `RMT` peripheral is chip-specific and should not be referenced in common code.
   
   Channel configuration should be performed at board level.



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961938900


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);

Review Comment:
   Yes, I agree.
   I've tryed to avoid it inside de interrupt routine since in those cases there is no need for atomic setting/clearing of the register's value. Also, it has some spinlock inside it. Isn't it a bit slower then getreg32+putreg32?
   I can change it if you insist, no hard feelings.
   
   



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961917770


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;
+      
+      if (dev->frame > 1)
+        dev->frame = 0;
+      
+      used_words = 0;
+      pixel_ptr = dev->data_ptr + current_pixel;
+  
+      while (used_words < dev->reload_thresh)
+        {
+          if (current_pixel < total_pixels)
+            {
+              register uint8_t pixel = (*pixel_ptr++);
+              for (register uint32_t i = 0; i < 8; i++)
+                {
+                  if (pixel & 0x80L)
+                    {
+                      putreg32(LOGIC_ONE, dst_mem);
+                    }
+                  else
+                    {
+                      putreg32(LOGIC_ZERO, dst_mem);
+                    }
+                  pixel <<= 1;
+                  dst_mem += 4;
+                }
+              used_words+=8;
+              current_pixel++;
+            }
+          else
+            {
+              regval = getreg32(RMT_INT_ENA_REG);
+              regval |= RMT_CHn_TX_END_INT_ENA(dev->channel);
+              putreg32(regval, RMT_INT_ENA_REG);
+              putreg32(0, dst_mem);
+              break;
+            }
+        }
+      
+      dev->next_pixel_to_load = current_pixel;
+    }
+  else
+    {
+      /* Perhaps an error, have no info on that */
+      putreg32(regval, RMT_INT_CLR_REG);
+    }
+  
+  return OK;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32_rmtinitialize
+ *
+ * Description:
+ *   Initialize the selected RMT device
+ *
+ * Input Parameters:
+ *   output_pin - the pin used for output
+ *   channel    - the RMT's channel that will be used
+ *
+ * Returned Value:
+ *   Valid RMT device structure reference on success; a NULL on failure
+ *
+ ****************************************************************************/
+
+struct rmt_dev_s *esp32_rmtinitialize(int output_pin, int channel)
+{
+  struct rmt_dev_s *rmtdev;
+  irqstate_t flags;
+
+  flags = enter_critical_section();

Review Comment:
   Nice to know that. 
   I've used the TWAI as reference, will change it to the the spinlock approach that the SPI uses. 



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#issuecomment-1240677383

   > Interrupt handler for the RMT driver should not restrict the usage to a single application.
   
   In the next days I will add a callback to the IRQ, as instructed, so the code that currently is inside the RMT's interruption will be moved to the WS2812 driver. This is allow the application specific logic to be kept outside the driver.


-- 
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

[GitHub] [nuttx] cederom commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "cederom (via GitHub)" <gi...@apache.org>.
cederom commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1646052040

   hello world i will take a look on Monday thank you! :-)
   
   --
   CeDeROM, SQ7MHZ, http://www.tomek.cedro.info
   


-- 
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

[GitHub] [nuttx] acassis commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1593812082

   @vbenso I nice think that I did with APA102 matrix was making a driver that see it as an ordinary LCD:
   https://github.com/apache/nuttx/blob/master/drivers/lcd/apa102.c
   
   Doing this way people could create big screens with WS2812 easily


-- 
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

[GitHub] [incubator-nuttx] acassis commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961908760


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;

Review Comment:
   ```suggestion:
   +      if (dev->frame++ == 1)
   +        {
   +          dst_mem += dev->reload_thresh * 4;
   +        }



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961957160


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;
+      
+      if (dev->frame > 1)
+        dev->frame = 0;
+      
+      used_words = 0;
+      pixel_ptr = dev->data_ptr + current_pixel;
+  
+      while (used_words < dev->reload_thresh)
+        {
+          if (current_pixel < total_pixels)
+            {
+              register uint8_t pixel = (*pixel_ptr++);
+              for (register uint32_t i = 0; i < 8; i++)
+                {
+                  if (pixel & 0x80L)
+                    {
+                      putreg32(LOGIC_ONE, dst_mem);
+                    }
+                  else
+                    {
+                      putreg32(LOGIC_ZERO, dst_mem);
+                    }
+                  pixel <<= 1;
+                  dst_mem += 4;
+                }
+              used_words+=8;
+              current_pixel++;
+            }
+          else
+            {
+              regval = getreg32(RMT_INT_ENA_REG);
+              regval |= RMT_CHn_TX_END_INT_ENA(dev->channel);
+              putreg32(regval, RMT_INT_ENA_REG);
+              putreg32(0, dst_mem);
+              break;
+            }
+        }
+      
+      dev->next_pixel_to_load = current_pixel;
+    }

Review Comment:
   The ws2812 is an 1-wire RGB LED that can be daisy chained with many others. Each LED receives a train of pulses encoding the 0's or 1's required to encode 24 bits of RGB color. Once the LED receives its pulses, it will adopt that color and only forward the remaining pulses to the next LED.
   Pulses for zeros and ones have an specific timing each, so LOGIC_ZERO and LOGIC_ONE will set the RMT to generate the output wave that will encode the desired bit in the output GPIO.
   Each channel of ESP32's RMT can hold up to 64 words of data, and we use one word to encode each pulse. This is enough for a single LED, however, when we have many LEDs connected to the system, there is the need to keep "refueling" the RMT's input with more data. 
   The RMT will generate an interrupt once it has transmitted half of the data on its internal buffer, so, when this happens, this routine will copy more data to its buffer. This process will happen until all the input data is copied to RMT's internal buffer. 
   In the last "refueling" interaction, 0 is written to RMT's buffer, and once RMT's transmission state machine reaches that point, it will go back to IDLE and generate an END interrupt, so that's when the write lock is released and the write function finally returns.
   
   Perhaps LOGIC_ONE and LOGIC_ZERO definitions' can be considered application specific and could be members of the dev structure. Also the term pixel, doesn't help much. I've considered translating the output bytes to the RMT encoding format inside the write function and then we would have no need to translate the bytes to pulse bits inside the interruption, however, it would require a lot of extra memory, since a single bit of input data, becomes a word when transmitting.



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r962154666


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;
+      
+      if (dev->frame > 1)
+        dev->frame = 0;
+      
+      used_words = 0;
+      pixel_ptr = dev->data_ptr + current_pixel;
+  
+      while (used_words < dev->reload_thresh)
+        {
+          if (current_pixel < total_pixels)
+            {
+              register uint8_t pixel = (*pixel_ptr++);
+              for (register uint32_t i = 0; i < 8; i++)
+                {
+                  if (pixel & 0x80L)
+                    {
+                      putreg32(LOGIC_ONE, dst_mem);
+                    }
+                  else
+                    {
+                      putreg32(LOGIC_ZERO, dst_mem);
+                    }
+                  pixel <<= 1;
+                  dst_mem += 4;
+                }
+              used_words+=8;
+              current_pixel++;
+            }
+          else
+            {
+              regval = getreg32(RMT_INT_ENA_REG);
+              regval |= RMT_CHn_TX_END_INT_ENA(dev->channel);
+              putreg32(regval, RMT_INT_ENA_REG);
+              putreg32(0, dst_mem);
+              break;
+            }
+        }
+      
+      dev->next_pixel_to_load = current_pixel;
+    }

Review Comment:
   Thanks for the explanation!
   
   > Perhaps LOGIC_ONE and LOGIC_ZERO definitions' can be considered application specific and could be members of the dev structure. Also the term pixel, doesn't help much. I've considered translating the output bytes to the RMT encoding format inside the write function and then we would have no need to translate the bytes to pulse bits inside the interruption, however, it would require a lot of extra memory, since a single bit of input data, becomes a word when transmitting.
   
   The issue is that this whole logic is application-specific, in this case for blinking LEDs controlled by the WS2812. But RMT could be used for different purposes. IDF, for example, provides several examples besides LED blinking: https://github.com/espressif/esp-idf/tree/master/examples/peripherals/rmt
   
   So this whole logic should not be present in the RMT code. Instead, it could be placed either at a device driver that uses the RMT or even at the application layer.
   See how the Timer driver provides an interface for registering a callback for handling timer expiration:
   https://github.com/apache/incubator-nuttx/blob/985710665a62e886694cf87f00be08fdf38e4702/arch/xtensa/src/esp32/esp32_tim_lowerhalf.c#L156-L179



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961997505


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);

Review Comment:
   > My mistake,
   > I was thinking that 1 was the highest priority. I'll change it to 1.
   
   No problem!
   
   > About the twai, I'll submit the PR later today.
   
   Thanks!



-- 
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

[GitHub] [nuttx] vbenso commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "vbenso (via GitHub)" <gi...@apache.org>.
vbenso commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1589601379

   > Hey there @vbenso :-) Did you manage do implement the updates? How can I help? I have ESP32 + WS2812 setup ready for testing / help in development :-)
   
   I thas been a while since I work with NuttX. I do remember submitting the PR, fixed many things and left the callback change for last, however couldn't manage to touch it since.
   I'll move the WS2812 dedicated code into the application. 
   And in the new solution, I think a callback registered from the user's app will translate the pixels to the RMT's time words. Being called by the RMT driver's IRQ every time it needs to refill it's buffer.
   
   
   > One question here: would it be possible to generate inverted pulses somehow with RMT so WS2812 gets signals over N-MOSFET transistor? It will invert signals but can interface 3.3V to 5V in case of devices that must have 5V DIO and does not accept direct 3.3V (like WS2812-MINI).
   
   With this approach the user's callback function will be able to adjust the timing and also de polarity of the output signal. The the RMT will be totally decoupled from the WS2812 and will be more generic, being able to generate other pulse sequences as well.
   
   I'll squeeze out some time this weekend for this changes and also the fix of some other ugly things that remain in this code.


-- 
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

[GitHub] [nuttx] acassis commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1646938584

   @vbenso @xiaoxiang781216 after merging it I noticed some details that escaped the review: I think the RMT Debug should be inside arch/xtensa/src/esp32/Kconfig instead of /Kconfig. What do you think?


-- 
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

[GitHub] [nuttx] vbenso commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "vbenso (via GitHub)" <gi...@apache.org>.
vbenso commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1605594887

   Hi everyone and @gustavonihei 
   I've just pushed another version of the RMT driver.
   Now it does only RMT transmition, all the processing related with the WS2812 was removed from the driver.
   USAGE:
    - The user opens the an RMT channel that is mapped to a GPIO pin. (/dev/rmtX for channel X);
    - User writes words (uint32_t) to this device;
    - The words are in the format described in the ESP32 Reference Manual 
     - - ((0x8000|tH)<<16) | (tL)) 
     -- Where tH is time in the high state and tL is time in low state of the channel's output.
     -- The units are based on the clock divider set for the RMT device, currently no divider, so APB's period.
    
    I'm considering the PR in the apps repository with a sample application @cederom .
    
    BTW: Is there any way to include the checkpatch script in vscode?


-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961872141


##########
boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c:
##########
@@ -0,0 +1,333 @@
+/****************************************************************************
+ * boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include "xtensa.h"
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/spi/spi.h>
+#include <nuttx/leds/ws2812.h>
+#include "esp32_rmt.h"
+
+//#include "stm32.h"
+//#include "stm32_spi.h"
+
+#ifdef CONFIG_WS2812
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#  ifndef CONFIG_WS2812_NON_SPI_DRIVER
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver.
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/leddrvN
+ *   spino - SPI port number
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+int board_ws2812_initialize(int devno, int spino, uint16_t nleds)
+{
+  struct spi_dev_s *spi;
+  char devpath[13];
+  int ret;
+
+  spi = esp32_spibus_initialize(spino);
+  if (spi == NULL)
+  {
+    return -ENODEV;
+  }
+
+  /* Register the WS2812 driver at the specified location. */
+
+  snprintf(devpath, 13, "/dev/leds%d", devno);
+  ret = ws2812_leds_register(devpath, spi, nleds);
+  if (ret < 0)
+  {
+    lederr("ERROR: ws2812_leds_register(%s) failed: %d\n",
+           devpath, ret);
+    return ret;
+  }
+
+  return OK;
+}
+#  else
+
+struct ws2812_rmt_s
+{
+  struct rmt_dev_s *rmt_dev;
+  size_t open_count;   /* Number of opens on this instance. */
+};
+
+static int ws2812_rmt_open(FAR struct file *filep)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  int ret;
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+  if (priv->open_count == 0)
+  {
+    ledinfo("rmt_dev=%p\n", priv->rmt_dev);
+    if (priv->rmt_dev)
+    {
+
+      uint32_t reg0_addr = RMT_CHnCONF0_REG(priv->rmt_dev->channel);
+      uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+      uint32_t reg_val = 0x00;
+
+      /* a single memory block with double buffering is enough */
+      uint32_t mem_blocks = 1; 
+      priv->rmt_dev->available_words = 64*mem_blocks;
+
+      uint32_t start_addr_chn = 0x3FF56800 + 64 * 4 * priv->rmt_dev->channel;
+      priv->rmt_dev->start_address = start_addr_chn;
+      
+
+      priv->rmt_dev->reload_thresh = priv->rmt_dev->available_words/2;
+
+      reg_val = (mem_blocks) << 24;
+      uint32_t clock_divider = 2;
+      reg_val |= (clock_divider);
+      putreg32(reg_val, reg0_addr);
+
+      reg_val = 0;
+      /* use APB clock */
+      reg_val |= RMT_REF_ALWAYS_ON_CHn; 
+      /* memory block in transmission mode */
+      reg_val &= ~RMT_MEM_OWNER_CHn;    
+      putreg32(reg_val, reg1_addr);
+
+      /* set when the buffer swapping IRQ must be generated */
+      uint32_t reload_addr = RMT_CHn_TX_LIM_REG(priv->rmt_dev->channel);
+      putreg32(priv->rmt_dev->reload_thresh, reload_addr);
+      
+      /* allow direct access to RMT's memory */
+      modifyreg32(RMT_APB_CONF_REG, 0, BIT(0)); 
+    }
+  }
+
+  priv->open_count += 1;
+
+  ret = OK;
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+static ssize_t ws2812_rmt_write(FAR struct file *filep,
+                        FAR const char *data,
+                        size_t len)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  if (data == NULL)
+  {
+    lederr("DATA is NULL\n");
+    return 0;
+  }
+  
+  nxsem_wait(&dev_data->exclsem);
+
+  uint32_t mem = priv->rmt_dev->start_address;
+  uint32_t used_words = 0;
+  uint32_t current_pixel = 0;
+  uint8_t *pixel_ptr = (uint8_t *)data;
+  uint8_t pixel;
+  uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+  modifyreg32(reg1_addr, 0, RMT_MEM_RD_RST_CHn);
+  modifyreg32(reg1_addr, RMT_MEM_RD_RST_CHn, 0);
+
+  while(used_words<priv->rmt_dev->available_words)
+    {
+      if (current_pixel < len)
+        {
+          pixel = *pixel_ptr++;
+          for (int i = 0; i < 8; i++)
+            {
+              int bit = pixel & 0x80L;
+              if (bit)
+                {
+                  putreg32(LOGIC_ONE, mem);
+                }
+              else
+               {
+                 putreg32(LOGIC_ZERO, mem);
+               }
+              mem += 4;
+              used_words++;
+              pixel <<= 1;
+            }
+          current_pixel++;
+        } 
+      else 
+        {
+          putreg32(0, mem); 
+          break;
+        }
+    }
+  
+  if(used_words<priv->rmt_dev->available_words) 
+    {
+      /* Small transactions won't need a the swap buffer IRQ */
+      modifyreg32(
+        RMT_INT_ENA_REG, 
+        RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel), 
+        RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel));
+    } 
+  else 
+    {
+    /* In large transactions inform the current state to the IRQ */
+    priv->rmt_dev->next_pixel_to_load = current_pixel;
+    priv->rmt_dev->data_ptr = (uint8_t*) data;
+    priv->rmt_dev->data_len = len;
+    priv->rmt_dev->frame = 0;
+    
+    /* Enable memory wrap around */
+    modifyreg32(RMT_APB_CONF_REG, 0, BIT(1));    
+
+    /* Enable buffering IRQ */
+    modifyreg32(
+      RMT_INT_ENA_REG, 
+      0, 
+      RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel) | 
+      RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel));
+  }
+
+  /* Start the transaction */
+  modifyreg32(reg1_addr, 0, RMT_TX_START_CHn);
+  nxsem_wait(&priv->rmt_dev->tx_sem);
+
+  /* Wait for the TX lock and release it after */
+  nxsem_wait(&priv->rmt_dev->tx_sem); 
+  nxsem_post(&priv->rmt_dev->tx_sem);
+  nxsem_post(&dev_data->exclsem);
+
+  return len;
+}
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver using the RMT 
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/ledsN
+ *   rmt_dev - Pointer to the RMT device that will be used
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int board_ws2812_initialize(int devno, uint16_t nleds, struct rmt_dev_s *rmt_dev)
+{
+  char devpath[13];
+  int ret;
+
+  FAR struct ws2812_dev_s *dev_data;

Review Comment:
   ```suggestion
     struct ws2812_dev_s *dev_data;
   ```
   `FAR` qualifier should not be used in arch-specific code.



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961898067


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;
+      
+      if (dev->frame > 1)
+        dev->frame = 0;
+      
+      used_words = 0;
+      pixel_ptr = dev->data_ptr + current_pixel;
+  
+      while (used_words < dev->reload_thresh)
+        {
+          if (current_pixel < total_pixels)
+            {
+              register uint8_t pixel = (*pixel_ptr++);
+              for (register uint32_t i = 0; i < 8; i++)
+                {
+                  if (pixel & 0x80L)
+                    {
+                      putreg32(LOGIC_ONE, dst_mem);
+                    }
+                  else
+                    {
+                      putreg32(LOGIC_ZERO, dst_mem);
+                    }
+                  pixel <<= 1;
+                  dst_mem += 4;
+                }
+              used_words+=8;
+              current_pixel++;
+            }
+          else
+            {
+              regval = getreg32(RMT_INT_ENA_REG);
+              regval |= RMT_CHn_TX_END_INT_ENA(dev->channel);
+              putreg32(regval, RMT_INT_ENA_REG);
+              putreg32(0, dst_mem);
+              break;
+            }
+        }
+      
+      dev->next_pixel_to_load = current_pixel;
+    }

Review Comment:
   I am not very familiar with the WS2812 nor the RMT peripheral.
   Could you please give a brief explanation of what this block is implementing?
   
   I have a feeling that this is application-specific logic and should not be present in driver code.



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961982748


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);

Review Comment:
   Since the initial motivation for setting the priority level as 2 no longer exists, let me ask again: what's the motivation for setting a priority level higher than **1** for the RMT interrupt?
   
   > In a side note, esp32twai_setup is calling:
   > `up_enable_irq(priv->cpuint);`
   
   It is also wrong.
   Could you please submit a new PR with the fix?



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961986233


##########
boards/xtensa/esp32/esp32-devkitc/src/esp32_bringup.c:
##########
@@ -162,91 +166,91 @@ int esp32_bringup(void)
 #ifdef CONFIG_ESP32_AES_ACCELERATOR
   ret = esp32_aes_init();
   if (ret < 0)
-    {
-      syslog(LOG_ERR,
-             "ERROR: Failed to initialize AES: %d\n", ret);
-    }
+  {

Review Comment:
   @hartmannathan 
   I've just learned about the nuttx/tools/checkpatch.sh and it seems to be using nxstyle in a batch manner. Can I use checkpath.sh instead of nxstyle to every-single-file?



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961894949


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);

Review Comment:
   ```suggestion
   
         modifyreg32(RMT_INT_ENA_REG, RMT_CHn_TX_END_INT_ENA(dev->channel), 0);
   ```
   `modifyreg32` can be used for achieving cleaner code.



-- 
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

[GitHub] [incubator-nuttx] hartmannathan commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961960719


##########
boards/xtensa/esp32/esp32-devkitc/src/esp32_bringup.c:
##########
@@ -162,91 +166,91 @@ int esp32_bringup(void)
 #ifdef CONFIG_ESP32_AES_ACCELERATOR
   ret = esp32_aes_init();
   if (ret < 0)
-    {
-      syslog(LOG_ERR,
-             "ERROR: Failed to initialize AES: %d\n", ret);
-    }
+  {

Review Comment:
   To check coding style, there is the program under tools/nxstyle.c. Just compile with:
   
   ```
   cd tools
   gcc nxstyle.c -o nxstyle
   cd ..
   ```
   
   and run it on the file in question, e.g.,
   
   ```
   $ tools/nxstyle boards/xtensa/esp32/esp32-devkitc/src/esp32_bringup.c
   ```
   
   It will print nothing if all is well or tell you which areas fail the check.



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961891907


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);

Review Comment:
   What's the motivation for setting a priority level of **2** here?



-- 
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

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961850389


##########
include/nuttx/board.h:
##########
@@ -837,6 +837,27 @@ void board_init_rngseed(void);
 int board_reset_cause(FAR struct boardioc_reset_cause_s *cause);
 #endif
 
+/****************************************************************************
+ * Name:  board_ws2812_initialize
+ *
+ * Description:
+ *   This function may called from application-specific logic during its
+ *   to perform board-specific initialization of the ws2812 device
+ *
+ *
+ ****************************************************************************/
+
+#  ifdef CONFIG_WS2812
+#    ifndef CONFIG_WS2812_NON_SPI_DRIVER
+int board_ws2812_initialize(int devno, int spino, uint16_t nleds);
+#    else
+int board_ws2812_initialize(
+    int devno, 
+    uint16_t nleds, 
+    struct rmt_dev_s *rmt_dev);

Review Comment:
   since rmt is specific for esp32, it's wrong to reference in the common header file.



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961874394


##########
boards/xtensa/esp32/esp32-devkitc/src/esp32_bringup.c:
##########
@@ -162,91 +166,91 @@ int esp32_bringup(void)
 #ifdef CONFIG_ESP32_AES_ACCELERATOR
   ret = esp32_aes_init();
   if (ret < 0)
-    {
-      syslog(LOG_ERR,
-             "ERROR: Failed to initialize AES: %d\n", ret);
-    }
+  {

Review Comment:
   This style changes are not compliant to the NuttX coding style.



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961881313


##########
boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c:
##########
@@ -0,0 +1,333 @@
+/****************************************************************************
+ * boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include "xtensa.h"
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/spi/spi.h>
+#include <nuttx/leds/ws2812.h>
+#include "esp32_rmt.h"
+
+//#include "stm32.h"
+//#include "stm32_spi.h"
+
+#ifdef CONFIG_WS2812
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#  ifndef CONFIG_WS2812_NON_SPI_DRIVER
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver.
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/leddrvN
+ *   spino - SPI port number
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+int board_ws2812_initialize(int devno, int spino, uint16_t nleds)
+{
+  struct spi_dev_s *spi;
+  char devpath[13];
+  int ret;
+
+  spi = esp32_spibus_initialize(spino);
+  if (spi == NULL)
+  {
+    return -ENODEV;
+  }
+
+  /* Register the WS2812 driver at the specified location. */
+
+  snprintf(devpath, 13, "/dev/leds%d", devno);
+  ret = ws2812_leds_register(devpath, spi, nleds);
+  if (ret < 0)
+  {
+    lederr("ERROR: ws2812_leds_register(%s) failed: %d\n",
+           devpath, ret);
+    return ret;
+  }
+
+  return OK;
+}
+#  else
+
+struct ws2812_rmt_s
+{
+  struct rmt_dev_s *rmt_dev;
+  size_t open_count;   /* Number of opens on this instance. */
+};
+
+static int ws2812_rmt_open(FAR struct file *filep)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  int ret;
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+  if (priv->open_count == 0)
+  {
+    ledinfo("rmt_dev=%p\n", priv->rmt_dev);
+    if (priv->rmt_dev)
+    {
+
+      uint32_t reg0_addr = RMT_CHnCONF0_REG(priv->rmt_dev->channel);
+      uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+      uint32_t reg_val = 0x00;
+
+      /* a single memory block with double buffering is enough */
+      uint32_t mem_blocks = 1; 
+      priv->rmt_dev->available_words = 64*mem_blocks;
+
+      uint32_t start_addr_chn = 0x3FF56800 + 64 * 4 * priv->rmt_dev->channel;
+      priv->rmt_dev->start_address = start_addr_chn;
+      
+
+      priv->rmt_dev->reload_thresh = priv->rmt_dev->available_words/2;
+
+      reg_val = (mem_blocks) << 24;
+      uint32_t clock_divider = 2;
+      reg_val |= (clock_divider);
+      putreg32(reg_val, reg0_addr);
+
+      reg_val = 0;
+      /* use APB clock */
+      reg_val |= RMT_REF_ALWAYS_ON_CHn; 
+      /* memory block in transmission mode */
+      reg_val &= ~RMT_MEM_OWNER_CHn;    
+      putreg32(reg_val, reg1_addr);
+
+      /* set when the buffer swapping IRQ must be generated */
+      uint32_t reload_addr = RMT_CHn_TX_LIM_REG(priv->rmt_dev->channel);
+      putreg32(priv->rmt_dev->reload_thresh, reload_addr);
+      
+      /* allow direct access to RMT's memory */
+      modifyreg32(RMT_APB_CONF_REG, 0, BIT(0)); 
+    }
+  }
+
+  priv->open_count += 1;
+
+  ret = OK;
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+static ssize_t ws2812_rmt_write(FAR struct file *filep,
+                        FAR const char *data,
+                        size_t len)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  if (data == NULL)
+  {
+    lederr("DATA is NULL\n");
+    return 0;
+  }
+  
+  nxsem_wait(&dev_data->exclsem);
+
+  uint32_t mem = priv->rmt_dev->start_address;
+  uint32_t used_words = 0;
+  uint32_t current_pixel = 0;
+  uint8_t *pixel_ptr = (uint8_t *)data;
+  uint8_t pixel;
+  uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+  modifyreg32(reg1_addr, 0, RMT_MEM_RD_RST_CHn);
+  modifyreg32(reg1_addr, RMT_MEM_RD_RST_CHn, 0);
+
+  while(used_words<priv->rmt_dev->available_words)
+    {
+      if (current_pixel < len)
+        {
+          pixel = *pixel_ptr++;
+          for (int i = 0; i < 8; i++)
+            {
+              int bit = pixel & 0x80L;
+              if (bit)
+                {
+                  putreg32(LOGIC_ONE, mem);
+                }
+              else
+               {
+                 putreg32(LOGIC_ZERO, mem);
+               }
+              mem += 4;
+              used_words++;
+              pixel <<= 1;
+            }
+          current_pixel++;
+        } 
+      else 
+        {
+          putreg32(0, mem); 
+          break;
+        }
+    }
+  
+  if(used_words<priv->rmt_dev->available_words) 
+    {
+      /* Small transactions won't need a the swap buffer IRQ */
+      modifyreg32(
+        RMT_INT_ENA_REG, 
+        RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel), 
+        RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel));
+    } 
+  else 
+    {
+    /* In large transactions inform the current state to the IRQ */
+    priv->rmt_dev->next_pixel_to_load = current_pixel;
+    priv->rmt_dev->data_ptr = (uint8_t*) data;
+    priv->rmt_dev->data_len = len;
+    priv->rmt_dev->frame = 0;
+    
+    /* Enable memory wrap around */
+    modifyreg32(RMT_APB_CONF_REG, 0, BIT(1));    
+
+    /* Enable buffering IRQ */
+    modifyreg32(
+      RMT_INT_ENA_REG, 
+      0, 
+      RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel) | 
+      RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel));
+  }
+
+  /* Start the transaction */
+  modifyreg32(reg1_addr, 0, RMT_TX_START_CHn);
+  nxsem_wait(&priv->rmt_dev->tx_sem);
+
+  /* Wait for the TX lock and release it after */
+  nxsem_wait(&priv->rmt_dev->tx_sem); 
+  nxsem_post(&priv->rmt_dev->tx_sem);
+  nxsem_post(&dev_data->exclsem);
+
+  return len;
+}
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver using the RMT 
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/ledsN
+ *   rmt_dev - Pointer to the RMT device that will be used
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int board_ws2812_initialize(int devno, uint16_t nleds, struct rmt_dev_s *rmt_dev)
+{
+  char devpath[13];
+  int ret;
+
+  FAR struct ws2812_dev_s *dev_data;
+
+  dev_data = kmm_zalloc(sizeof(struct ws2812_dev_s));
+
+  if (dev_data == NULL)
+    {
+      set_errno(ENOMEM);
+      return -ENOMEM;
+    }
+
+  /* Allocate struct holding out persistent data */
+
+  struct ws2812_rmt_s *priv = kmm_zalloc(sizeof(struct ws2812_rmt_s));
+  priv->rmt_dev = rmt_dev;
+
+  dev_data->open = ws2812_rmt_open;
+  dev_data->write = ws2812_rmt_write;
+  dev_data->close = NULL;
+  dev_data->read = NULL;
+
+  dev_data->nleds = nleds;
+  dev_data->clock = CONFIG_WS2812_FREQUENCY;
+  dev_data->private = priv;
+
+  nxsem_init(&dev_data->exclsem, 0, 1);
+
+  /* Register the WS2812 driver at the specified location. */
+  snprintf(devpath, 13, "/dev/leds%d", devno);

Review Comment:
   ```suggestion
     snprintf(devpath, sizeof(devpath), "/dev/leds%d", devno);
   ```



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961892484


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)

Review Comment:
   Also, prototype for this function is missing in the section above.



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961957160


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;
+      
+      if (dev->frame > 1)
+        dev->frame = 0;
+      
+      used_words = 0;
+      pixel_ptr = dev->data_ptr + current_pixel;
+  
+      while (used_words < dev->reload_thresh)
+        {
+          if (current_pixel < total_pixels)
+            {
+              register uint8_t pixel = (*pixel_ptr++);
+              for (register uint32_t i = 0; i < 8; i++)
+                {
+                  if (pixel & 0x80L)
+                    {
+                      putreg32(LOGIC_ONE, dst_mem);
+                    }
+                  else
+                    {
+                      putreg32(LOGIC_ZERO, dst_mem);
+                    }
+                  pixel <<= 1;
+                  dst_mem += 4;
+                }
+              used_words+=8;
+              current_pixel++;
+            }
+          else
+            {
+              regval = getreg32(RMT_INT_ENA_REG);
+              regval |= RMT_CHn_TX_END_INT_ENA(dev->channel);
+              putreg32(regval, RMT_INT_ENA_REG);
+              putreg32(0, dst_mem);
+              break;
+            }
+        }
+      
+      dev->next_pixel_to_load = current_pixel;
+    }

Review Comment:
   The ws2812 is an one-wire (doesn't use the 1-wire protocol, but comunicates over a single wire nevertheless) RGB LED that can be daisy chained with many others. Each LED receives a train of pulses encoding the 0's or 1's required to encode 24 bits of RGB color. Once the LED receives its pulses, it will adopt that color and only forward the remaining pulses to the next LED.
   Pulses for zeros and ones have an specific timing each, so LOGIC_ZERO and LOGIC_ONE will set the RMT to generate the output wave that will encode the desired bit in the output GPIO.
   Each channel of ESP32's RMT can hold up to 64 words of data, and we use one word to encode each pulse. This is enough for a single LED, however, when we have many LEDs connected to the system, there is the need to keep "refueling" the RMT's input with more data. 
   The RMT will generate an interrupt once it has transmitted half of the data on its internal buffer, so, when this happens, this routine will copy more data to its buffer. This process will happen until all the input data is copied to RMT's internal buffer. 
   In the last "refueling" interaction, 0 is written to RMT's buffer, and once RMT's transmission state machine reaches that point, it will go back to IDLE and generate an END interrupt, so that's when the write lock is released and the write function finally returns.
   
   Perhaps LOGIC_ONE and LOGIC_ZERO definitions' can be considered application specific and could be members of the dev structure. Also the term pixel, doesn't help much. I've considered translating the output bytes to the RMT encoding format inside the write function and then we would have no need to translate the bytes to pulse bits inside the interruption, however, it would require a lot of extra memory, since a single bit of input data, becomes a word when transmitting.



-- 
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

[GitHub] [nuttx] xiaoxiang781216 commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1646012374

   > Hi @acassis @cederom Is this ok?
   
   please rebase your change to resolve the conflict.
   


-- 
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

[GitHub] [nuttx] vbenso commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "vbenso (via GitHub)" <gi...@apache.org>.
vbenso commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1605595331

   I can see that I've done something wrong with the git flow. Too many files changed. 
   Any contribution about how to fix this will be very welcome.
   


-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961879513


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);
+  if (dev->cpuint < 0)
+    {
+      /* Failed to allocate a CPU interrupt of this type. */
+  
+      ret = dev->cpuint;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+  
+  ret = irq_attach(dev->irq, esp32rmt_interrupt, dev);
+  
+  if (ret != OK)
+    {
+      /* Failed to attach IRQ, so CPU interrupt must be freed. */
+  
+      esp32_teardown_irq(dev->cpu, dev->periph, dev->cpuint);
+      dev->cpuint = -ENOMEM;
+      leave_critical_section(flags);
+  
+      return ret;
+    }
+
+  /* Enable the CPU interrupt that is linked to the RMT device. */
+  up_enable_irq(dev->cpuint);
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+/****************************************************************************
+ * Name: esp32rmt_interrupt
+ *
+ * Description:
+ *   RMT TX interrupt handler
+ *
+ * Input Parameters:
+ *   irq - The IRQ number of the interrupt.
+ *   context - The register state save array at the time of the interrupt.
+ *   arg - The pointer to driver structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg)
+{
+  struct rmt_dev_s *dev = (struct rmt_dev_s *)arg;
+  uint32_t regval = getreg32(RMT_INT_ST_REG);
+  uint32_t total_pixels;
+  uint32_t current_pixel;
+  uint32_t dst_mem;
+  uint32_t used_words;
+  uint8_t *pixel_ptr;
+  
+  if (regval & RMT_CHn_TX_END_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_END_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      regval = getreg32(RMT_INT_ENA_REG);
+      regval &= ~(RMT_CHn_TX_END_INT_ENA(dev->channel));
+      putreg32(regval, RMT_INT_ENA_REG);
+      nxsem_post(&dev->tx_sem);
+    }
+  else if (regval & RMT_CHn_TX_THR_EVENT_INT_ST(dev->channel))
+    {
+      putreg32(RMT_CHn_TX_THR_EVENT_INT_CLR(dev->channel), RMT_INT_CLR_REG);
+      total_pixels = dev->data_len;
+      current_pixel = dev->next_pixel_to_load;
+      dst_mem = dev->start_address;
+      if(dev->frame++==1)
+        dst_mem += dev->reload_thresh * 4;
+      
+      if (dev->frame > 1)
+        dev->frame = 0;
+      
+      used_words = 0;
+      pixel_ptr = dev->data_ptr + current_pixel;
+  
+      while (used_words < dev->reload_thresh)
+        {
+          if (current_pixel < total_pixels)
+            {
+              register uint8_t pixel = (*pixel_ptr++);
+              for (register uint32_t i = 0; i < 8; i++)
+                {
+                  if (pixel & 0x80L)
+                    {
+                      putreg32(LOGIC_ONE, dst_mem);
+                    }
+                  else
+                    {
+                      putreg32(LOGIC_ZERO, dst_mem);
+                    }
+                  pixel <<= 1;
+                  dst_mem += 4;
+                }
+              used_words+=8;
+              current_pixel++;
+            }
+          else
+            {
+              regval = getreg32(RMT_INT_ENA_REG);
+              regval |= RMT_CHn_TX_END_INT_ENA(dev->channel);
+              putreg32(regval, RMT_INT_ENA_REG);
+              putreg32(0, dst_mem);
+              break;
+            }
+        }
+      
+      dev->next_pixel_to_load = current_pixel;
+    }
+  else
+    {
+      /* Perhaps an error, have no info on that */
+      putreg32(regval, RMT_INT_CLR_REG);
+    }
+  
+  return OK;
+}
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32_rmtinitialize
+ *
+ * Description:
+ *   Initialize the selected RMT device
+ *
+ * Input Parameters:
+ *   output_pin - the pin used for output
+ *   channel    - the RMT's channel that will be used
+ *
+ * Returned Value:
+ *   Valid RMT device structure reference on success; a NULL on failure
+ *
+ ****************************************************************************/
+
+struct rmt_dev_s *esp32_rmtinitialize(int output_pin, int channel)
+{
+  struct rmt_dev_s *rmtdev;
+  irqstate_t flags;
+
+  flags = enter_critical_section();

Review Comment:
   For ensuring correctness of this driver when SMP is enabled, we should use the `spin_lock_irqsave`/`spin_lock_irqrestore` APIs with a device-specific lock.
   When SMP is enabled,  `enter_critical_section` disables the interrupts only in the currently executing core, thus it does not prevent the other cores from accessing the critical section.
   
   Please refer to other peripheral drivers from ESP32 (e.g. SPI).



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961883956


##########
include/nuttx/board.h:
##########
@@ -837,6 +837,27 @@ void board_init_rngseed(void);
 int board_reset_cause(FAR struct boardioc_reset_cause_s *cause);
 #endif
 
+/****************************************************************************
+ * Name:  board_ws2812_initialize
+ *
+ * Description:
+ *   This function may called from application-specific logic during its
+ *   to perform board-specific initialization of the ws2812 device
+ *
+ *
+ ****************************************************************************/
+
+#  ifdef CONFIG_WS2812
+#    ifndef CONFIG_WS2812_NON_SPI_DRIVER
+int board_ws2812_initialize(int devno, int spino, uint16_t nleds);
+#    else
+int board_ws2812_initialize(
+    int devno, 
+    uint16_t nleds, 
+    struct rmt_dev_s *rmt_dev);

Review Comment:
   They are defined on `esp32-devkitc.h` header as well, so these may be simply removed.



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961887605


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)

Review Comment:
   ```suggestion
   static int rmt_setup(struct rmt_dev_s *dev)
   ```
   Since this function is private thus restricted to ESP32, the `esp32_` prefix may be omitted.



-- 
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

[GitHub] [incubator-nuttx] vbenso commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
vbenso commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961972888


##########
boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c:
##########
@@ -0,0 +1,333 @@
+/****************************************************************************
+ * boards/xtensa/esp32/esp32-devkitc/src/esp32_ws2812.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <errno.h>
+#include <debug.h>
+#include <stdio.h>
+
+#include "xtensa.h"
+
+#include <nuttx/kmalloc.h>
+#include <nuttx/spi/spi.h>
+#include <nuttx/leds/ws2812.h>
+#include "esp32_rmt.h"
+
+//#include "stm32.h"
+//#include "stm32_spi.h"
+
+#ifdef CONFIG_WS2812
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+#  ifndef CONFIG_WS2812_NON_SPI_DRIVER
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver.
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/leddrvN
+ *   spino - SPI port number
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+int board_ws2812_initialize(int devno, int spino, uint16_t nleds)
+{
+  struct spi_dev_s *spi;
+  char devpath[13];
+  int ret;
+
+  spi = esp32_spibus_initialize(spino);
+  if (spi == NULL)
+  {
+    return -ENODEV;
+  }
+
+  /* Register the WS2812 driver at the specified location. */
+
+  snprintf(devpath, 13, "/dev/leds%d", devno);
+  ret = ws2812_leds_register(devpath, spi, nleds);
+  if (ret < 0)
+  {
+    lederr("ERROR: ws2812_leds_register(%s) failed: %d\n",
+           devpath, ret);
+    return ret;
+  }
+
+  return OK;
+}
+#  else
+
+struct ws2812_rmt_s
+{
+  struct rmt_dev_s *rmt_dev;
+  size_t open_count;   /* Number of opens on this instance. */
+};
+
+static int ws2812_rmt_open(FAR struct file *filep)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  int ret;
+  irqstate_t flags;
+
+  flags = enter_critical_section();
+  if (priv->open_count == 0)
+  {
+    ledinfo("rmt_dev=%p\n", priv->rmt_dev);
+    if (priv->rmt_dev)
+    {
+
+      uint32_t reg0_addr = RMT_CHnCONF0_REG(priv->rmt_dev->channel);
+      uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+      uint32_t reg_val = 0x00;
+
+      /* a single memory block with double buffering is enough */
+      uint32_t mem_blocks = 1; 
+      priv->rmt_dev->available_words = 64*mem_blocks;
+
+      uint32_t start_addr_chn = 0x3FF56800 + 64 * 4 * priv->rmt_dev->channel;
+      priv->rmt_dev->start_address = start_addr_chn;
+      
+
+      priv->rmt_dev->reload_thresh = priv->rmt_dev->available_words/2;
+
+      reg_val = (mem_blocks) << 24;
+      uint32_t clock_divider = 2;
+      reg_val |= (clock_divider);
+      putreg32(reg_val, reg0_addr);
+
+      reg_val = 0;
+      /* use APB clock */
+      reg_val |= RMT_REF_ALWAYS_ON_CHn; 
+      /* memory block in transmission mode */
+      reg_val &= ~RMT_MEM_OWNER_CHn;    
+      putreg32(reg_val, reg1_addr);
+
+      /* set when the buffer swapping IRQ must be generated */
+      uint32_t reload_addr = RMT_CHn_TX_LIM_REG(priv->rmt_dev->channel);
+      putreg32(priv->rmt_dev->reload_thresh, reload_addr);
+      
+      /* allow direct access to RMT's memory */
+      modifyreg32(RMT_APB_CONF_REG, 0, BIT(0)); 
+    }
+  }
+
+  priv->open_count += 1;
+
+  ret = OK;
+
+  leave_critical_section(flags);
+
+  return ret;
+}
+
+static ssize_t ws2812_rmt_write(FAR struct file *filep,
+                        FAR const char *data,
+                        size_t len)
+{
+  FAR struct inode *inode = filep->f_inode;
+  FAR struct ws2812_dev_s *dev_data = inode->i_private;
+  FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
+                                      dev_data->private;
+  if (data == NULL)
+  {
+    lederr("DATA is NULL\n");
+    return 0;
+  }
+  
+  nxsem_wait(&dev_data->exclsem);
+
+  uint32_t mem = priv->rmt_dev->start_address;
+  uint32_t used_words = 0;
+  uint32_t current_pixel = 0;
+  uint8_t *pixel_ptr = (uint8_t *)data;
+  uint8_t pixel;
+  uint32_t reg1_addr = RMT_CHnCONF1_REG(priv->rmt_dev->channel);
+  modifyreg32(reg1_addr, 0, RMT_MEM_RD_RST_CHn);
+  modifyreg32(reg1_addr, RMT_MEM_RD_RST_CHn, 0);
+
+  while(used_words<priv->rmt_dev->available_words)
+    {
+      if (current_pixel < len)
+        {
+          pixel = *pixel_ptr++;
+          for (int i = 0; i < 8; i++)
+            {
+              int bit = pixel & 0x80L;
+              if (bit)
+                {
+                  putreg32(LOGIC_ONE, mem);
+                }
+              else
+               {
+                 putreg32(LOGIC_ZERO, mem);
+               }
+              mem += 4;
+              used_words++;
+              pixel <<= 1;
+            }
+          current_pixel++;
+        } 
+      else 
+        {
+          putreg32(0, mem); 
+          break;
+        }
+    }
+  
+  if(used_words<priv->rmt_dev->available_words) 
+    {
+      /* Small transactions won't need a the swap buffer IRQ */
+      modifyreg32(
+        RMT_INT_ENA_REG, 
+        RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel), 
+        RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel));
+    } 
+  else 
+    {
+    /* In large transactions inform the current state to the IRQ */
+    priv->rmt_dev->next_pixel_to_load = current_pixel;
+    priv->rmt_dev->data_ptr = (uint8_t*) data;
+    priv->rmt_dev->data_len = len;
+    priv->rmt_dev->frame = 0;
+    
+    /* Enable memory wrap around */
+    modifyreg32(RMT_APB_CONF_REG, 0, BIT(1));    
+
+    /* Enable buffering IRQ */
+    modifyreg32(
+      RMT_INT_ENA_REG, 
+      0, 
+      RMT_CHn_TX_END_INT_ENA(priv->rmt_dev->channel) | 
+      RMT_CHn_TX_THR_EVENT_INT_ENA(priv->rmt_dev->channel));
+  }
+
+  /* Start the transaction */
+  modifyreg32(reg1_addr, 0, RMT_TX_START_CHn);
+  nxsem_wait(&priv->rmt_dev->tx_sem);
+
+  /* Wait for the TX lock and release it after */
+  nxsem_wait(&priv->rmt_dev->tx_sem); 
+  nxsem_post(&priv->rmt_dev->tx_sem);
+  nxsem_post(&dev_data->exclsem);
+
+  return len;
+}
+/****************************************************************************
+ * Name: board_ws2812_initialize
+ *
+ * Description:
+ *   Initialize and register the WS2812 LED driver using the RMT 
+ *
+ * Input Parameters:
+ *   devno - The device number, used to build the device path as /dev/ledsN
+ *   rmt_dev - Pointer to the RMT device that will be used
+ *   nleds - number of LEDs
+ *
+ * Returned Value:
+ *   Zero (OK) on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+int board_ws2812_initialize(int devno, uint16_t nleds, struct rmt_dev_s *rmt_dev)
+{
+  char devpath[13];
+  int ret;
+
+  FAR struct ws2812_dev_s *dev_data;

Review Comment:
   Are these supposed to be removed as well?
   
   ```
   static int ws2812_rmt_open(FAR struct file *filep)
   {
     FAR struct inode *inode = filep->f_inode;
     FAR struct ws2812_dev_s *dev_data = inode->i_private;
     FAR struct ws2812_rmt_s *priv = (FAR struct ws2812_rmt_s *)
                                         dev_data->private;
   
   ```



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r961982748


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,315 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void esp32rmt_reset(struct rmt_dev_s *dev);
+
+IRAM_ATTR static int esp32rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: esp32rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void esp32rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret;
+  
+  flags = enter_critical_section();
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xFFFFFFFF, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  leave_critical_section(flags);
+}
+
+/****************************************************************************
+ * Name: esp32rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int esp32rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = enter_critical_section();
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+      up_disable_irq(dev->cpuint);
+    }
+
+  dev->cpu = up_cpu_index();
+  dev->cpuint = esp32_setup_irq(dev->cpu, dev->periph,
+                                2, ESP32_CPUINT_LEVEL);

Review Comment:
   Since the initial motivation for setting the priority level as 2 no longer exists, let me ask again: what's the motivation for setting a priority level higher than **1**?
   
   > In a side note, esp32twai_setup is calling:
   > `up_enable_irq(priv->cpuint);`
   
   It is also wrong.
   Could you please submit a new PR with the fix?



-- 
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

[GitHub] [incubator-nuttx] gustavonihei commented on a diff in pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by GitBox <gi...@apache.org>.
gustavonihei commented on code in PR #6992:
URL: https://github.com/apache/incubator-nuttx/pull/6992#discussion_r962154702


##########
arch/xtensa/src/esp32/esp32_rmt.c:
##########
@@ -0,0 +1,322 @@
+/****************************************************************************
+ * arch/xtensa/src/esp32/esp32_rmt.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <stdio.h>
+#include <sys/types.h>
+#include <inttypes.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/kmalloc.h>
+#include <arch/board/board.h>
+#include <nuttx/irq.h>
+#include <nuttx/arch.h>
+#include <nuttx/spinlock.h>
+
+#include "xtensa.h"
+
+#include "esp32_gpio.h"
+#include "esp32_rmt.h"
+#include "esp32_irq.h"
+#include "esp32_clockconfig.h"
+
+#include "hardware/esp32_dport.h"
+#include "hardware/esp32_gpio_sigmap.h"
+
+#ifdef CONFIG_ESP32_RMT
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+/* RMT methods */
+
+static void rmt_reset(struct rmt_dev_s *dev);
+static int rmt_setup(struct rmt_dev_s *dev);
+IRAM_ATTR static int rmt_interrupt(int irq, void *context, void *arg);
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+struct rmt_dev_s g_rmt_dev =
+{
+  .channel = 0,
+  .periph  = ESP32_PERIPH_RMT,
+  .irq     = ESP32_IRQ_RMT,
+  .cpu     = 0,
+  .cpuint  = -ENOMEM,
+  .lock    = SP_UNLOCKED
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: rmt_reset
+ *
+ * Description:
+ *   Reset the RMT device.  Called early to initialize the hardware. This
+ *   function is called, before esp32_rmt_setup().
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *  None
+ *
+ ****************************************************************************/
+
+static void rmt_reset(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+
+  flags = spin_lock_irqsave(&dev->lock);
+
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 1);
+  modifyreg32(DPORT_PERIP_RST_EN_REG, DPORT_RMT_RST, 0);
+  putreg32(0xffffffff, RMT_INT_CLR_REG); /* Clear any spurious IRQ Flag */
+
+  spin_unlock_irqrestore(&dev->lock, flags);
+}
+
+/****************************************************************************
+ * Name: rmt_setup
+ *
+ * Description:
+ *   Configure the RMT. This method is called the first time that the RMT
+ *   device is opened.  This will occur when the port is first opened.
+ *   This setup includes configuring and attaching RMT interrupts.
+ *
+ * Input Parameters:
+ *   dev - An instance of the "upper half" RMT driver state structure.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno on failure
+ *
+ ****************************************************************************/
+
+static int rmt_setup(struct rmt_dev_s *dev)
+{
+  irqstate_t flags;
+  int ret = OK;
+
+  flags = spin_lock_irqsave(&dev->lock);
+
+  if (dev->cpuint != -ENOMEM)
+    {
+      /* Disable the provided CPU Interrupt to configure it. */
+
+      up_disable_irq(dev->cpuint);

Review Comment:
   ```suggestion
         up_disable_irq(dev->irq);
   ```



-- 
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

[GitHub] [nuttx] xiaoxiang781216 commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1647084005

   > @vbenso @xiaoxiang781216 after merging it I noticed some details that escaped the review: I think the RMT Debug should be inside arch/xtensa/src/esp32/Kconfig instead of /Kconfig. What do you think?
   
   Yes, it's specific to eps32.


-- 
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

[GitHub] [nuttx] acassis commented on pull request #6992: WS2812 LED driver using ESP32's RMT peripheral

Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #6992:
URL: https://github.com/apache/nuttx/pull/6992#issuecomment-1637150835

   @vbenso please fix the conflicts and update the PR to let us merge this PR


-- 
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