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 2020/08/20 03:17:03 UTC

[GitHub] [incubator-nuttx] donghengqaz opened a new pull request #1610: xtensa/esp32: Improve SPI transmission

donghengqaz opened a new pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610


   This driver refactoring was implemented by Dong Heng dongheng@espressif.com
   and reviewed by Alan Carvalho de Assis.
   
   ## Summary
   
   Improve SPI transmission.
   
   ## Impact
   
   Master:
     1. add DMA RX/TX support
     2. add software chip selection
     3. add user defined chip selection
     4. add IOMUX check and IO map
   
   Slave:
     1. add DMA RX/TX support
     2. add IOMUX check and IO map
     3. use full 256 bit SPI TX/RX cache in non-DMA mode
   
   ## Testing
   
   Use SPI master and slave example to test between 2 ESP2 core boards.
   


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

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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r473909668



##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -158,6 +190,20 @@ static void esp32_spi_deinit(FAR struct spi_dev_s *dev);
  * Public Function Prototypes
  ****************************************************************************/
 
+extern uint8_t esp32_spi2_status(FAR struct spi_dev_s *dev, uint32_t devid);
+extern uint8_t esp32_spi3_status(FAR struct spi_dev_s *dev, uint32_t devid);
+
+/* Use board level CS select function */
+
+#ifdef CONFIG_ESP32_SPI_UDCS
+extern void esp32_spi2_select(FAR struct spi_dev_s *dev,
+                              uint32_t devid,
+                              bool selected);
+extern void esp32_spi3_select(FAR struct spi_dev_s *dev,
+                              uint32_t devid,
+                              bool selected);
+#endif
+

Review comment:
       I will remove these including `esp32_spi2_status` and `esp32_spi3_status`, I see they all prototyped in the 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.

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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r474372984



##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -46,18 +46,33 @@
 #include "esp32_spi.h"
 #include "esp32_gpio.h"
 #include "esp32_cpuint.h"
+#include "esp32_dma.h"
 
 #include "xtensa.h"
 #include "hardware/esp32_gpio_sigmap.h"
 #include "hardware/esp32_dport.h"
 #include "hardware/esp32_spi.h"
 #include "hardware/esp32_soc.h"
+#include "hardware/esp32_pinmap.h"
 #include "rom/esp32_gpio.h"
 
 /****************************************************************************
  * Private Types
  ****************************************************************************/
 
+/* SPI DMA hardware channel number */
+
+#define SPI_DMA_CHAN      (2)
+
+/* SPI DMA RX/TX description number */
+
+#define SPI_DMADESC_NUM   (CONFIG_SPI_DMADESC_NUM)

Review comment:
       The driver adds EOF and "next=NULL" into to the last DMA description to tell hardware that this is the last node, so that we don't need to add en extra one.

##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -46,18 +46,33 @@
 #include "esp32_spi.h"
 #include "esp32_gpio.h"
 #include "esp32_cpuint.h"
+#include "esp32_dma.h"
 
 #include "xtensa.h"
 #include "hardware/esp32_gpio_sigmap.h"
 #include "hardware/esp32_dport.h"
 #include "hardware/esp32_spi.h"
 #include "hardware/esp32_soc.h"
+#include "hardware/esp32_pinmap.h"
 #include "rom/esp32_gpio.h"
 
 /****************************************************************************
  * Private Types
  ****************************************************************************/
 
+/* SPI DMA hardware channel number */
+
+#define SPI_DMA_CHAN      (2)
+
+/* SPI DMA RX/TX description number */
+
+#define SPI_DMADESC_NUM   (CONFIG_SPI_DMADESC_NUM)

Review comment:
       The driver adds EOF and "next=NULL" into to the last DMA description to tell hardware that this is the last node, so that we don't need to add an extra one.




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

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



[GitHub] [incubator-nuttx] donghengqaz commented on pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#issuecomment-678147136


   > LGTM, thanks @donghengqaz.
   
   @Ouss4 Thanks for your deeply reviewing.
   


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

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



[GitHub] [incubator-nuttx] Ouss4 merged pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
Ouss4 merged pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610


   


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

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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r473742541



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -362,11 +362,53 @@ endmenu # I2C configuration
 menu "SPI configuration"
 	depends on ESP32_SPI
 
+config ESP32_SPI_SWCS
+	bool "SPI software CS"
+	default y
+	---help---
+		Use SPI software CS.
+
+config ESP32_SPI_UDCS
+	bool "User defined CS"
+	default n
+	depends on ESP32_SPI_SWCS
+	---help---
+		Use user defined CS.
+
+choice
+	prompt "DMA owner"
+	default ESP32_SPI2_DMA
+	depends on ESP32_SPI2 || ESP32_SPI3
+	---help---
+		Select which SPI owns DMA, only one SPI DMA is available, so
+		only one SPI can use DMA.

Review comment:
       By this you mean DMA controller or DMA channels?
   From the manual:
   >two DMA channels are shared by SPI1, SPI2 and SPI3 controllers. Each DMA channel can be used by
   any one SPI controller at any given time.
   
   It looks like the code expects to have only one channel (channel 2) available.  Am I looking at an old document?




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

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



[GitHub] [incubator-nuttx] yamt commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r561729131



##########
File path: arch/xtensa/src/esp32/esp32_spi_slave.c
##########
@@ -548,36 +754,78 @@ static int esp32_io_interrupt(int irq, void *context, FAR void *arg)
 
 static int esp32_spislv_interrupt(int irq, void *context, FAR void *arg)
 {
-  uint32_t rd;
-  uint32_t wd;
   struct esp32_spislv_priv_s *priv = (struct esp32_spislv_priv_s *)arg;
-  irqstate_t flags;
-
-  flags = enter_critical_section();
+  uint32_t n;
+  uint32_t tmp;
+  int i;
 
   esp32_spi_reset_regbits(priv, SPI_SLAVE_OFFSET, SPI_TRANS_DONE_M);
 
-  if (!priv->process)
+  if (priv->dma_chan)
+    {
+      esp32_spi_set_reg(priv, SPI_DMA_CONF_OFFSET, SPI_DMA_RESET_MASK);
+      esp32_spi_reset_regbits(priv, SPI_DMA_CONF_OFFSET,
+                              SPI_DMA_RESET_MASK);
+    }
+
+  if (priv->process == false)
     {
       SPI_SDEV_SELECT(priv->sdev, true);
       priv->process = true;
     }
 
-  rd = esp32_spi_get_reg(priv, SPI_W0_OFFSET);
-  if (spi_dequeue(priv, &wd))
+  /* Read and calculate read bytes */
+
+  n = (esp32_spi_get_reg(priv, SPI_SLV_RD_BIT_OFFSET) + 1) / 8;
+
+  esp32_spi_set_regbits(priv, SPI_USER_OFFSET, SPI_USR_MOSI_M);
+
+  /* RX process */
+
+  if (!priv->dma_chan)
     {
-      esp32_spi_set_reg(priv, SPI_W0_OFFSET, wd);
-      esp32_spi_set_regbits(priv, SPI_CMD_OFFSET, SPI_USR_M);
+      /** With DMA, software should copy data from regitser
+       *  to receive buffer
+       */
+
+      for (i = 0; i < n; i += 4)
+        {
+          tmp = esp32_spi_get_reg(priv, SPI_W0_OFFSET + i);
+          memcpy(priv->rxbuffer + priv->rxlen + i, &tmp, n);
+        }
     }
 
-  SPI_SDEV_RECEIVE(priv->sdev, &rd, 1);
+  priv->rxlen += n;
 
-  if (priv->process == false)
+  esp32_spislv_rx(priv);
+
+  /* TX process */
+
+  if (priv->txen)
     {
-      SPI_SDEV_SELECT(priv->sdev, false);
+      if (n < priv->txlen)
+        {
+          priv->txlen -= n;

Review comment:
       to me, it looks weird to use the same `n` value read from SPI_SLV_RD_BIT_OFFSET for both of rx and tx.
   is this intended?
   




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

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



[GitHub] [incubator-nuttx] donghengqaz commented on pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#issuecomment-677387676


   @acassis @patacongo @Ouss4 PTAL.


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

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



[GitHub] [incubator-nuttx] yamt commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
yamt commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r561729131



##########
File path: arch/xtensa/src/esp32/esp32_spi_slave.c
##########
@@ -548,36 +754,78 @@ static int esp32_io_interrupt(int irq, void *context, FAR void *arg)
 
 static int esp32_spislv_interrupt(int irq, void *context, FAR void *arg)
 {
-  uint32_t rd;
-  uint32_t wd;
   struct esp32_spislv_priv_s *priv = (struct esp32_spislv_priv_s *)arg;
-  irqstate_t flags;
-
-  flags = enter_critical_section();
+  uint32_t n;
+  uint32_t tmp;
+  int i;
 
   esp32_spi_reset_regbits(priv, SPI_SLAVE_OFFSET, SPI_TRANS_DONE_M);
 
-  if (!priv->process)
+  if (priv->dma_chan)
+    {
+      esp32_spi_set_reg(priv, SPI_DMA_CONF_OFFSET, SPI_DMA_RESET_MASK);
+      esp32_spi_reset_regbits(priv, SPI_DMA_CONF_OFFSET,
+                              SPI_DMA_RESET_MASK);
+    }
+
+  if (priv->process == false)
     {
       SPI_SDEV_SELECT(priv->sdev, true);
       priv->process = true;
     }
 
-  rd = esp32_spi_get_reg(priv, SPI_W0_OFFSET);
-  if (spi_dequeue(priv, &wd))
+  /* Read and calculate read bytes */
+
+  n = (esp32_spi_get_reg(priv, SPI_SLV_RD_BIT_OFFSET) + 1) / 8;
+
+  esp32_spi_set_regbits(priv, SPI_USER_OFFSET, SPI_USR_MOSI_M);
+
+  /* RX process */
+
+  if (!priv->dma_chan)
     {
-      esp32_spi_set_reg(priv, SPI_W0_OFFSET, wd);
-      esp32_spi_set_regbits(priv, SPI_CMD_OFFSET, SPI_USR_M);
+      /** With DMA, software should copy data from regitser
+       *  to receive buffer
+       */
+
+      for (i = 0; i < n; i += 4)
+        {
+          tmp = esp32_spi_get_reg(priv, SPI_W0_OFFSET + i);
+          memcpy(priv->rxbuffer + priv->rxlen + i, &tmp, n);
+        }
     }
 
-  SPI_SDEV_RECEIVE(priv->sdev, &rd, 1);
+  priv->rxlen += n;
 
-  if (priv->process == false)
+  esp32_spislv_rx(priv);
+
+  /* TX process */
+
+  if (priv->txen)
     {
-      SPI_SDEV_SELECT(priv->sdev, false);
+      if (n < priv->txlen)
+        {
+          priv->txlen -= n;

Review comment:
       to me, it looks weird to use the same `n` value read from SPI_SLV_RD_BIT_OFFSET for both of rx and tx.
   is this intended?
   




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

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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r474372984



##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -46,18 +46,33 @@
 #include "esp32_spi.h"
 #include "esp32_gpio.h"
 #include "esp32_cpuint.h"
+#include "esp32_dma.h"
 
 #include "xtensa.h"
 #include "hardware/esp32_gpio_sigmap.h"
 #include "hardware/esp32_dport.h"
 #include "hardware/esp32_spi.h"
 #include "hardware/esp32_soc.h"
+#include "hardware/esp32_pinmap.h"
 #include "rom/esp32_gpio.h"
 
 /****************************************************************************
  * Private Types
  ****************************************************************************/
 
+/* SPI DMA hardware channel number */
+
+#define SPI_DMA_CHAN      (2)
+
+/* SPI DMA RX/TX description number */
+
+#define SPI_DMADESC_NUM   (CONFIG_SPI_DMADESC_NUM)

Review comment:
       The driver add EOF and "next=NULL" into to the last DMA description to tell hardware that this is the last node, so that we need to add en extra one.

##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -46,18 +46,33 @@
 #include "esp32_spi.h"
 #include "esp32_gpio.h"
 #include "esp32_cpuint.h"
+#include "esp32_dma.h"
 
 #include "xtensa.h"
 #include "hardware/esp32_gpio_sigmap.h"
 #include "hardware/esp32_dport.h"
 #include "hardware/esp32_spi.h"
 #include "hardware/esp32_soc.h"
+#include "hardware/esp32_pinmap.h"
 #include "rom/esp32_gpio.h"
 
 /****************************************************************************
  * Private Types
  ****************************************************************************/
 
+/* SPI DMA hardware channel number */
+
+#define SPI_DMA_CHAN      (2)
+
+/* SPI DMA RX/TX description number */
+
+#define SPI_DMADESC_NUM   (CONFIG_SPI_DMADESC_NUM)

Review comment:
       The driver adds EOF and "next=NULL" into to the last DMA description to tell hardware that this is the last node, so that we need to add en extra one.




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

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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r473925458



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -362,11 +362,53 @@ endmenu # I2C configuration
 menu "SPI configuration"
 	depends on ESP32_SPI
 
+config ESP32_SPI_SWCS
+	bool "SPI software CS"
+	default y
+	---help---
+		Use SPI software CS.
+
+config ESP32_SPI_UDCS
+	bool "User defined CS"
+	default n
+	depends on ESP32_SPI_SWCS
+	---help---
+		Use user defined CS.
+
+choice
+	prompt "DMA owner"
+	default ESP32_SPI2_DMA
+	depends on ESP32_SPI2 || ESP32_SPI3
+	---help---
+		Select which SPI owns DMA, only one SPI DMA is available, so
+		only one SPI can use DMA.

Review comment:
       But maybe letting SPI2 and SPI3 use both these DMA channel is better design.




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

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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r473700716



##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -695,13 +1005,72 @@ static uint32_t esp32_spi_send(FAR struct spi_dev_s *dev, uint32_t wd)
 }
 
 /****************************************************************************
- * Name: esp32_spi_exchange
+ * Name: esp32_spi_send

Review comment:
       ```suggestion
    * Name: esp32_spi_dma_send
   ```

##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -158,6 +190,20 @@ static void esp32_spi_deinit(FAR struct spi_dev_s *dev);
  * Public Function Prototypes
  ****************************************************************************/
 
+extern uint8_t esp32_spi2_status(FAR struct spi_dev_s *dev, uint32_t devid);
+extern uint8_t esp32_spi3_status(FAR struct spi_dev_s *dev, uint32_t devid);
+
+/* Use board level CS select function */
+
+#ifdef CONFIG_ESP32_SPI_UDCS
+extern void esp32_spi2_select(FAR struct spi_dev_s *dev,
+                              uint32_t devid,
+                              bool selected);
+extern void esp32_spi3_select(FAR struct spi_dev_s *dev,
+                              uint32_t devid,
+                              bool selected);
+#endif
+

Review comment:
       These are already prototyped in the 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.

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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r473972836



##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -46,18 +46,33 @@
 #include "esp32_spi.h"
 #include "esp32_gpio.h"
 #include "esp32_cpuint.h"
+#include "esp32_dma.h"
 
 #include "xtensa.h"
 #include "hardware/esp32_gpio_sigmap.h"
 #include "hardware/esp32_dport.h"
 #include "hardware/esp32_spi.h"
 #include "hardware/esp32_soc.h"
+#include "hardware/esp32_pinmap.h"
 #include "rom/esp32_gpio.h"
 
 /****************************************************************************
  * Private Types
  ****************************************************************************/
 
+/* SPI DMA hardware channel number */
+
+#define SPI_DMA_CHAN      (2)
+
+/* SPI DMA RX/TX description number */
+
+#define SPI_DMADESC_NUM   (CONFIG_SPI_DMADESC_NUM)

Review comment:
       ```suggestion
   #define SPI_DMADESC_NUM   (CONFIG_SPI_DMADESC_NUM + 1)
   ```
   Shouldn't be +1 for the last descriptor?




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

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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r473924509



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -362,11 +362,53 @@ endmenu # I2C configuration
 menu "SPI configuration"
 	depends on ESP32_SPI
 
+config ESP32_SPI_SWCS
+	bool "SPI software CS"
+	default y
+	---help---
+		Use SPI software CS.
+
+config ESP32_SPI_UDCS
+	bool "User defined CS"
+	default n
+	depends on ESP32_SPI_SWCS
+	---help---
+		Use user defined CS.
+
+choice
+	prompt "DMA owner"
+	default ESP32_SPI2_DMA
+	depends on ESP32_SPI2 || ESP32_SPI3
+	---help---
+		Select which SPI owns DMA, only one SPI DMA is available, so
+		only one SPI can use DMA.

Review comment:
       Yes, you are right. I mean DMA channel here, not just DMA.
   
   ESP32 has 4 SPI hardware controllers, SPI0 for code flash, SPI1 is a little different from SPI2 and SPI3, because it has an arbiter connecting to IO, add its I/O pins are same as SPI0, so I have not added SPI1 driver.
   
   My original idea was keeping a channel for SPI1, and driver only uses one channel. So it is what it is now.




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

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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r473910226



##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -695,13 +1005,72 @@ static uint32_t esp32_spi_send(FAR struct spi_dev_s *dev, uint32_t wd)
 }
 
 /****************************************************************************
- * Name: esp32_spi_exchange
+ * Name: esp32_spi_send

Review comment:
       Thanks for your deeply reviewing.




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

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



[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r473945259



##########
File path: arch/xtensa/src/esp32/Kconfig
##########
@@ -362,11 +362,53 @@ endmenu # I2C configuration
 menu "SPI configuration"
 	depends on ESP32_SPI
 
+config ESP32_SPI_SWCS
+	bool "SPI software CS"
+	default y
+	---help---
+		Use SPI software CS.
+
+config ESP32_SPI_UDCS
+	bool "User defined CS"
+	default n
+	depends on ESP32_SPI_SWCS
+	---help---
+		Use user defined CS.
+
+choice
+	prompt "DMA owner"
+	default ESP32_SPI2_DMA
+	depends on ESP32_SPI2 || ESP32_SPI3
+	---help---
+		Select which SPI owns DMA, only one SPI DMA is available, so
+		only one SPI can use DMA.

Review comment:
       Ah ok, you wanted to dedicate one DMA channel for SPI1.  I think we can still manage that through Kconfig and have the two channels available for SPI2 and SPI3 in case SPI1 is not used, but I understand if this is how things are set now and can be improved for another iteration.




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

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



[GitHub] [incubator-nuttx] donghengqaz commented on a change in pull request #1610: xtensa/esp32: Improve SPI transmission

Posted by GitBox <gi...@apache.org>.
donghengqaz commented on a change in pull request #1610:
URL: https://github.com/apache/incubator-nuttx/pull/1610#discussion_r473909668



##########
File path: arch/xtensa/src/esp32/esp32_spi.c
##########
@@ -158,6 +190,20 @@ static void esp32_spi_deinit(FAR struct spi_dev_s *dev);
  * Public Function Prototypes
  ****************************************************************************/
 
+extern uint8_t esp32_spi2_status(FAR struct spi_dev_s *dev, uint32_t devid);
+extern uint8_t esp32_spi3_status(FAR struct spi_dev_s *dev, uint32_t devid);
+
+/* Use board level CS select function */
+
+#ifdef CONFIG_ESP32_SPI_UDCS
+extern void esp32_spi2_select(FAR struct spi_dev_s *dev,
+                              uint32_t devid,
+                              bool selected);
+extern void esp32_spi3_select(FAR struct spi_dev_s *dev,
+                              uint32_t devid,
+                              bool selected);
+#endif
+

Review comment:
       I will remove these including `esp32_spi2_status` and `esp32_spi3_status`, I see they are both prototyped in the 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.

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