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/04/03 11:27:17 UTC

[GitHub] [incubator-nuttx] davids5 opened a new pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

davids5 opened a new pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706
 
 
   This add an optional ability to have buffering in the SPI driver for DMA. 
   
   If a SPI Bus uses an "in driver buffers" we will incur 2 copies,  The copy cost is << less the non DMA transfer time and having the buffer in the driver ensures DMA can be used. 
   
   This is needed for because the SPI API does not support passing the buffer extent, so the only extent is buffer + the transfer size.  This is an issue on H7 with caching. These can sizes be less than the cache line size, and not aligned and typically greater then 4 bytes, which is about the break even point for the DMA IO overhead.
   
   
   The SPI driver was violating the DMA/SPI setup as described in 49.4.14 Communication using DMA (direct memory addressing) of RM0433 Rev 6. The result was that the TX DMA operation would always return FIFO overrun/underrun (FEIFx) indicating a FIFO error event occurred on the stream.
   
   This PR fixes this issue.
   
   

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#discussion_r403016610
 
 

 ##########
 File path: arch/arm/src/stm32h7/stm32_spi.c
 ##########
 @@ -1768,23 +1840,48 @@ static void spi_exchange(FAR struct spi_dev_s *dev, FAR const void *txbuffer,
     }
 
 #ifdef CONFIG_STM32H7_DMACAPABLE
-  stm32_dmacfg_t dmacfg1;
-  stm32_dmacfg_t dmacfg2;
+  stm32_dmacfg_t dmatxcfg;
+  stm32_dmacfg_t dmarxcfg;
+
+  /* Setup DMAs */
+
+  /* If this bus uses a in driver buffers we will incur 2 copies,
+   * The copy cost is << less the non DMA transfer time and having
+   * the buffer in the driver ensures DMA can be used. This is bacause
+   * the API does not support passing the buffer extent so the only
+   * extent is buffer + the transfer size. These can sizes be less than
+   * the cache line size, and not aligned and tyicaly greater then 4
+   * bytes, which is about the break even point for the DMA IO overhead.
+   */
+
+  if (txbuffer && priv->txbuf)
+    {
+      if (nbytes > priv->buflen)
 
 Review comment:
   The API does allow for returning an error.  We could change that but in real use the assumption is the allocation is the worst case for the system on that bus.  

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#discussion_r403019222
 
 

 ##########
 File path: arch/arm/src/stm32h7/stm32_spi.c
 ##########
 @@ -1768,23 +1840,48 @@ static void spi_exchange(FAR struct spi_dev_s *dev, FAR const void *txbuffer,
     }
 
 #ifdef CONFIG_STM32H7_DMACAPABLE
-  stm32_dmacfg_t dmacfg1;
-  stm32_dmacfg_t dmacfg2;
+  stm32_dmacfg_t dmatxcfg;
+  stm32_dmacfg_t dmarxcfg;
+
+  /* Setup DMAs */
+
+  /* If this bus uses a in driver buffers we will incur 2 copies,
+   * The copy cost is << less the non DMA transfer time and having
+   * the buffer in the driver ensures DMA can be used. This is bacause
+   * the API does not support passing the buffer extent so the only
+   * extent is buffer + the transfer size. These can sizes be less than
+   * the cache line size, and not aligned and tyicaly greater then 4
+   * bytes, which is about the break even point for the DMA IO overhead.
+   */
+
+  if (txbuffer && priv->txbuf)
+    {
+      if (nbytes > priv->buflen)
 
 Review comment:
   Instead of returning error, it's better to:
   1.Send the buffer segment by segment or
   2.Switch to PIO mode

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#discussion_r403026086
 
 

 ##########
 File path: arch/arm/src/stm32h7/stm32_spi.c
 ##########
 @@ -1768,23 +1840,48 @@ static void spi_exchange(FAR struct spi_dev_s *dev, FAR const void *txbuffer,
     }
 
 #ifdef CONFIG_STM32H7_DMACAPABLE
-  stm32_dmacfg_t dmacfg1;
-  stm32_dmacfg_t dmacfg2;
+  stm32_dmacfg_t dmatxcfg;
+  stm32_dmacfg_t dmarxcfg;
+
+  /* Setup DMAs */
+
+  /* If this bus uses a in driver buffers we will incur 2 copies,
+   * The copy cost is << less the non DMA transfer time and having
+   * the buffer in the driver ensures DMA can be used. This is bacause
+   * the API does not support passing the buffer extent so the only
+   * extent is buffer + the transfer size. These can sizes be less than
+   * the cache line size, and not aligned and tyicaly greater then 4
+   * bytes, which is about the break even point for the DMA IO overhead.
+   */
+
+  if (txbuffer && priv->txbuf)
+    {
+      if (nbytes > priv->buflen)
 
 Review comment:
   @xiaoxiang781216 - thanks - force pushed using truncated value in DMA setup

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] Ouss4 commented on issue #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
Ouss4 commented on issue #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#issuecomment-608433572
 
 
   @davids5  With a dummy RX transfer, aren't you invalidating more data from the cache then needed?
   That's of course not harmful, but probably not what's desired.

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#discussion_r403010908
 
 

 ##########
 File path: arch/arm/src/stm32h7/stm32_spi.c
 ##########
 @@ -1768,23 +1840,48 @@ static void spi_exchange(FAR struct spi_dev_s *dev, FAR const void *txbuffer,
     }
 
 #ifdef CONFIG_STM32H7_DMACAPABLE
-  stm32_dmacfg_t dmacfg1;
-  stm32_dmacfg_t dmacfg2;
+  stm32_dmacfg_t dmatxcfg;
+  stm32_dmacfg_t dmarxcfg;
+
+  /* Setup DMAs */
+
+  /* If this bus uses a in driver buffers we will incur 2 copies,
+   * The copy cost is << less the non DMA transfer time and having
+   * the buffer in the driver ensures DMA can be used. This is bacause
+   * the API does not support passing the buffer extent so the only
+   * extent is buffer + the transfer size. These can sizes be less than
+   * the cache line size, and not aligned and tyicaly greater then 4
+   * bytes, which is about the break even point for the DMA IO overhead.
+   */
+
+  if (txbuffer && priv->txbuf)
+    {
+      if (nbytes > priv->buflen)
 
 Review comment:
   Is it safe to truncatethe buffer?

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] Ouss4 edited a comment on issue #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
Ouss4 edited a comment on issue #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#issuecomment-608433572
 
 
   @davids5  With a dummy RX transfer, aren't you invalidating more data from the cache than needed?
   That's of course not harmful, but probably not what's desired.

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#discussion_r402946084
 
 

 ##########
 File path: arch/arm/src/stm32h7/stm32_spi.c
 ##########
 @@ -489,9 +577,14 @@ static struct stm32_spidev_s g_spi4dev =
 #ifdef CONFIG_STM32H7_SPI_INTERRUPTS
   .spiirq   = STM32_IRQ_SPI4,
 #endif
-#ifdef CONFIG_STM32H7_SPI_DMA
+#ifdef CONFIG_STM32H7_SPI3_DMA
 
 Review comment:
   ```suggestion
   #ifdef CONFIG_STM32H7_SPI4_DMA
   ```

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] davids5 commented on issue #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
davids5 commented on issue #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#issuecomment-608447197
 
 
   > @davids5 With a dummy RX transfer, aren't you invalidating more data from the cache than needed?
   > That's of course not harmful, but probably not what's desired.
   
   I am not seeing it.  When rxbuffer is null - there is no invalidation. 
   

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#discussion_r403036968
 
 

 ##########
 File path: arch/arm/src/stm32h7/stm32_spi.c
 ##########
 @@ -1768,23 +1840,48 @@ static void spi_exchange(FAR struct spi_dev_s *dev, FAR const void *txbuffer,
     }
 
 #ifdef CONFIG_STM32H7_DMACAPABLE
-  stm32_dmacfg_t dmacfg1;
-  stm32_dmacfg_t dmacfg2;
+  stm32_dmacfg_t dmatxcfg;
+  stm32_dmacfg_t dmarxcfg;
+
+  /* Setup DMAs */
+
+  /* If this bus uses a in driver buffers we will incur 2 copies,
+   * The copy cost is << less the non DMA transfer time and having
+   * the buffer in the driver ensures DMA can be used. This is bacause
+   * the API does not support passing the buffer extent so the only
+   * extent is buffer + the transfer size. These can sizes be less than
+   * the cache line size, and not aligned and tyicaly greater then 4
+   * bytes, which is about the break even point for the DMA IO overhead.
+   */
+
+  if (txbuffer && priv->txbuf)
+    {
+      if (nbytes > priv->buflen)
 
 Review comment:
   @xiaoxiang781216 - I would be better. But this is for performance on small packets. The assumption is the worst case size is set for the bus. PIO would defeat the purpose. Looping would also. 
   

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706#discussion_r403018565
 
 

 ##########
 File path: arch/arm/src/stm32h7/stm32_spi.c
 ##########
 @@ -1768,23 +1840,48 @@ static void spi_exchange(FAR struct spi_dev_s *dev, FAR const void *txbuffer,
     }
 
 #ifdef CONFIG_STM32H7_DMACAPABLE
-  stm32_dmacfg_t dmacfg1;
-  stm32_dmacfg_t dmacfg2;
+  stm32_dmacfg_t dmatxcfg;
+  stm32_dmacfg_t dmarxcfg;
+
+  /* Setup DMAs */
+
+  /* If this bus uses a in driver buffers we will incur 2 copies,
+   * The copy cost is << less the non DMA transfer time and having
+   * the buffer in the driver ensures DMA can be used. This is bacause
+   * the API does not support passing the buffer extent so the only
+   * extent is buffer + the transfer size. These can sizes be less than
+   * the cache line size, and not aligned and tyicaly greater then 4
+   * bytes, which is about the break even point for the DMA IO overhead.
+   */
+
+  if (txbuffer && priv->txbuf)
+    {
+      if (nbytes > priv->buflen)
 
 Review comment:
   The error is further down. I will force push....

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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo merged pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA

Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #706: stm32h7:SPI Add buffers for DMA & and FIX DMA
URL: https://github.com/apache/incubator-nuttx/pull/706
 
 
   

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


With regards,
Apache Git Services