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/11/14 18:47:28 UTC

[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7596: esp32/i2s: implement I2S receiver module

pkarashchenko commented on code in PR #7596:
URL: https://github.com/apache/incubator-nuttx/pull/7596#discussion_r1021933752


##########
boards/xtensa/esp32/common/src/esp32_board_i2sdev.c:
##########
@@ -54,58 +54,101 @@
  *   number.
  *
  * Input Parameters:
- *   port  - The I2S port used for the device
+ *   port       - The I2S port used for the device
+ *   enable_tx  - Register device as TX if true
+ *   enable_rx  - Register device as RX if true
  *
  * Returned Value:
  *   Zero is returned on success.  Otherwise, a negated errno value is
  *   returned to indicate the nature of the failure.
  *
  ****************************************************************************/
 
-int board_i2sdev_initialize(int port)
+int board_i2sdev_initialize(int port, bool enable_tx, bool enable_rx)
 {
   struct audio_lowerhalf_s *audio_i2s;
-  struct audio_lowerhalf_s *pcm;
   struct i2s_dev_s *i2s;
-  char devname[12];
+  char devname[8];
   int ret;
 
   ainfo("Initializing I2S\n");
 
   i2s = esp32_i2sbus_initialize(port);
 
 #ifdef CONFIG_AUDIO_I2SCHAR
-  ret = i2schar_register(i2s, 0);
+  ret = i2schar_register(i2s, port);
   if (ret < 0)
     {
       aerr("ERROR: i2schar_register failed: %d\n", ret);
       return ret;
     }
 #endif
 
-  audio_i2s = audio_i2s_initialize(i2s, true);
-
-  if (!audio_i2s)
+  if (enable_tx)
     {
-      auderr("ERROR: Failed to initialize I2S\n");
-      return -ENODEV;
-    }
+      /* Initialize audio output */
 
-  pcm = pcm_decode_initialize(audio_i2s);
+      audio_i2s = audio_i2s_initialize(i2s, true);
 
-  if (!pcm)
-    {
-      auderr("ERROR: Failed create the PCM decoder\n");
-      return  -ENODEV;
-    }
+      if (!audio_i2s)
+        {
+          auderr("ERROR: Failed to initialize I2S audio output\n");
+          return -ENODEV;
+        }
 
-  snprintf(devname, 12, "pcm%d", port);
+      snprintf(devname, sizeof(devname), "pcm%d", port);
 
-  ret = audio_register(devname, pcm);
+      /* If nxlooper is selected, the playback buffer is not rendered as
+       * a WAV file. Therefore, PCM decode will fail while processing such
+       * output buffer. In such a case, we bypass the PCM decode.
+       */
 
-  if (ret < 0)
+#ifdef CONFIG_SYSTEM_NXLOOPER
+      ret = audio_register(devname, audio_i2s);
+#else
+      struct audio_lowerhalf_s *pcm;
+
+      pcm = pcm_decode_initialize(audio_i2s);
+
+      if (!pcm)
+        {
+          auderr("ERROR: Failed create the PCM decoder\n");
+          return -ENODEV;
+        }
+
+      ret = audio_register(devname, pcm);
+#endif /* CONFIG_SYSTEM_NXLOOPER */
+
+      if (ret < 0)
+        {
+          auderr("ERROR: Failed to register /dev/%s device: %d\n",
+                 devname, ret);
+          return ret;
+        }
+    }
+
+  if (enable_rx)
     {
-      auderr("ERROR: Failed to register /dev/%s device: %d\n", devname, ret);
+      /* Initialize audio input */
+
+      audio_i2s = audio_i2s_initialize(i2s, false);
+
+      if (!audio_i2s)

Review Comment:
   ```suggestion
         if (audio_i2s == NULL)
   ```
   



##########
boards/xtensa/esp32/common/src/esp32_board_i2sdev.c:
##########
@@ -54,58 +54,101 @@
  *   number.
  *
  * Input Parameters:
- *   port  - The I2S port used for the device
+ *   port       - The I2S port used for the device
+ *   enable_tx  - Register device as TX if true
+ *   enable_rx  - Register device as RX if true
  *
  * Returned Value:
  *   Zero is returned on success.  Otherwise, a negated errno value is
  *   returned to indicate the nature of the failure.
  *
  ****************************************************************************/
 
-int board_i2sdev_initialize(int port)
+int board_i2sdev_initialize(int port, bool enable_tx, bool enable_rx)
 {
   struct audio_lowerhalf_s *audio_i2s;
-  struct audio_lowerhalf_s *pcm;
   struct i2s_dev_s *i2s;
-  char devname[12];
+  char devname[8];
   int ret;
 
   ainfo("Initializing I2S\n");
 
   i2s = esp32_i2sbus_initialize(port);
 
 #ifdef CONFIG_AUDIO_I2SCHAR
-  ret = i2schar_register(i2s, 0);
+  ret = i2schar_register(i2s, port);
   if (ret < 0)
     {
       aerr("ERROR: i2schar_register failed: %d\n", ret);
       return ret;
     }
 #endif
 
-  audio_i2s = audio_i2s_initialize(i2s, true);
-
-  if (!audio_i2s)
+  if (enable_tx)
     {
-      auderr("ERROR: Failed to initialize I2S\n");
-      return -ENODEV;
-    }
+      /* Initialize audio output */
 
-  pcm = pcm_decode_initialize(audio_i2s);
+      audio_i2s = audio_i2s_initialize(i2s, true);
 
-  if (!pcm)
-    {
-      auderr("ERROR: Failed create the PCM decoder\n");
-      return  -ENODEV;
-    }
+      if (!audio_i2s)
+        {
+          auderr("ERROR: Failed to initialize I2S audio output\n");
+          return -ENODEV;
+        }
 
-  snprintf(devname, 12, "pcm%d", port);
+      snprintf(devname, sizeof(devname), "pcm%d", port);
 
-  ret = audio_register(devname, pcm);
+      /* If nxlooper is selected, the playback buffer is not rendered as
+       * a WAV file. Therefore, PCM decode will fail while processing such
+       * output buffer. In such a case, we bypass the PCM decode.
+       */
 
-  if (ret < 0)
+#ifdef CONFIG_SYSTEM_NXLOOPER
+      ret = audio_register(devname, audio_i2s);
+#else
+      struct audio_lowerhalf_s *pcm;
+
+      pcm = pcm_decode_initialize(audio_i2s);
+
+      if (!pcm)

Review Comment:
   ```suggestion
         if (pcm == NULL)
   ```
   



##########
boards/xtensa/esp32/common/src/esp32_board_i2sdev.c:
##########
@@ -54,58 +54,101 @@
  *   number.
  *
  * Input Parameters:
- *   port  - The I2S port used for the device
+ *   port       - The I2S port used for the device
+ *   enable_tx  - Register device as TX if true
+ *   enable_rx  - Register device as RX if true
  *
  * Returned Value:
  *   Zero is returned on success.  Otherwise, a negated errno value is
  *   returned to indicate the nature of the failure.
  *
  ****************************************************************************/
 
-int board_i2sdev_initialize(int port)
+int board_i2sdev_initialize(int port, bool enable_tx, bool enable_rx)
 {
   struct audio_lowerhalf_s *audio_i2s;
-  struct audio_lowerhalf_s *pcm;
   struct i2s_dev_s *i2s;
-  char devname[12];
+  char devname[8];
   int ret;
 
   ainfo("Initializing I2S\n");
 
   i2s = esp32_i2sbus_initialize(port);
 
 #ifdef CONFIG_AUDIO_I2SCHAR
-  ret = i2schar_register(i2s, 0);
+  ret = i2schar_register(i2s, port);
   if (ret < 0)
     {
       aerr("ERROR: i2schar_register failed: %d\n", ret);
       return ret;
     }
 #endif
 
-  audio_i2s = audio_i2s_initialize(i2s, true);
-
-  if (!audio_i2s)
+  if (enable_tx)
     {
-      auderr("ERROR: Failed to initialize I2S\n");
-      return -ENODEV;
-    }
+      /* Initialize audio output */
 
-  pcm = pcm_decode_initialize(audio_i2s);
+      audio_i2s = audio_i2s_initialize(i2s, true);
 
-  if (!pcm)
-    {
-      auderr("ERROR: Failed create the PCM decoder\n");
-      return  -ENODEV;
-    }
+      if (!audio_i2s)

Review Comment:
   ```suggestion
         if (audio_i2s == NULL)
   ```
   



##########
arch/xtensa/src/esp32/esp32_i2s.c:
##########
@@ -808,6 +918,77 @@ static IRAM_ATTR int i2s_txdma_setup(struct esp32_i2s_s *priv,
 }
 #endif /* I2S_HAVE_TX */
 
+/****************************************************************************
+ * Name: i2s_rxdma_setup
+ *
+ * Description:
+ *   Setup the next RX DMA transfer
+ *
+ * Input Parameters:
+ *   priv - Initialized I2S device structure.
+ *   bfcontainer - The buffer container to be set up
+ *
+ * Returned Value:
+ *   OK on success; a negated errno value on failure
+ *
+ * Assumptions:
+ *   Interrupts are disabled
+ *
+ ****************************************************************************/
+
+#ifdef I2S_HAVE_RX
+static int i2s_rxdma_setup(struct esp32_i2s_s *priv,
+                           struct esp32_buffer_s *bfcontainer)
+{
+  int ret = OK;
+  struct esp32_dmadesc_s *inlink;
+  uint32_t bytes_queued;
+  irqstate_t flags;
+
+  DEBUGASSERT(bfcontainer && bfcontainer->apb);
+
+  inlink = bfcontainer->dma_link;
+
+  /* Allocate the internal buffer for RX */
+
+  bfcontainer->buf = (uint8_t *)calloc(bfcontainer->nbytes, 1);

Review Comment:
   ```suggestion
     bfcontainer->buf = calloc(bfcontainer->nbytes, 1);
   ```
   



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