WIP: STM32F7 DMA per SPI #25

dagar · 2018-07-09T22:03:32Z

Targeting PX4's NuttX branch for development and testing, but the intention is to upstream this.

davids5 · 2018-07-09T23:31:21Z

arch/arm/src/stm32/stm32_spi.c

+  if (!(priv->rxch && priv->txch) ||
+      (txbuffer && !stm32_dmacapable((uint32_t)txbuffer, nwords, priv->txccr)) ||
+      (rxbuffer && !stm32_dmacapable((uint32_t)rxbuffer, nwords, priv->rxccr)) ||
+      up_interrupt_context())


Is up_interrupt_context() is a PX4 specific thing, but not a general usage?

I see it all over nuttx proper, although if it seems out of place here we could do something on the PX4 side to force it. I think that will be important for finding a way to do this incrementally.

@dagar - I think it is an artifact of calling the SPI off the HRT. We will need to change that to upstream it.

@dagar the more i think about it this is correct. A driver using SPI in interrupt context can not do the sem_wait.

davids5 · 2018-07-09T23:34:23Z

arch/arm/src/stm32/Kconfig

@@ -6381,6 +6381,48 @@ config STM32_SPI_DMA
 	---help---
 		Use DMA to improve SPI transfer performance.  Cannot be used with STM32_SPI_INTERRUPT.

+config STM32_SPI1_DMA


Do we want to have TX and RX granularity on the DMA? TX are usually tiny. The DMA setup would take longer then the TX to the bus.

I was wondering about that, we'd have to further split spi_exchange.

On second thought I don't think it makes any sense to split it because of the way SPI works we really need 2 DMA per SPI

LorenzMeier · 2018-07-10T05:48:47Z

@davids5 Could you please support this?

LorenzMeier · 2018-07-10T05:49:02Z

@thomasgubler FYI

dagar · 2018-08-20T00:57:20Z

@davids5 review?

dagar · 2018-09-11T16:59:55Z

Rebased on current px4_firmware_nuttx-7.22+

thomasgubler · 2018-09-24T16:31:12Z

This will be picked up once there is functionality (PR) in PX4 that allows to test it

dakejahl · 2018-10-17T15:41:05Z

This has been successfully tested on SPI1 in PX4/PX4-Autopilot#10602

davids5

@dagar Looks Good. Just Coding standards. 4.4 for nested #ifdef and if then else Statement

davids5 · 2018-10-17T22:40:13Z

arch/arm/src/stm32/stm32_spi.c

@@ -290,8 +290,13 @@ static struct stm32_spidev_s g_spi1dev =
  .spiirq   = STM32_IRQ_SPI1,
 #endif
 #ifdef CONFIG_STM32_SPI_DMA
+#ifdef CONFIG_STM32_SPI1_DMA


Indent the nested #if def

#ifdef CONFIG_ABC # ifdef CONFIG_DEF

davids5 · 2018-10-17T22:45:27Z

arch/arm/src/stm32/stm32_spi.c

+  if (!(priv->rxch && priv->txch) ||
+      (txbuffer && !stm32_dmacapable((uint32_t)txbuffer, nwords, priv->txccr)) ||
+      (rxbuffer && !stm32_dmacapable((uint32_t)rxbuffer, nwords, priv->rxccr)) ||
+      up_interrupt_context())


@dagar the more i think about it this is correct. A driver using SPI in interrupt context can not do the sem_wait.

davids5 · 2018-10-17T22:49:35Z

arch/arm/src/stm32/stm32_spi.c

@@ -1730,25 +1762,31 @@ static void spi_bus_initialize(FAR struct stm32_spidev_s *priv)
   * priority inheritance enabled.
   */

-  sem_init(&priv->rxsem, 0, 0);
-  sem_init(&priv->txsem, 0, 0);
+  if (priv->rxch && priv->txch) {


Coding standards. 4.4 if then else Statement (here after NCS)
http://nuttx.org/doku.php?id=documentation:codingstandard

davids5 · 2018-10-17T22:49:43Z

arch/arm/src/stm32/stm32_spi.c


-  spi_modifycr2(priv, SPI_CR2_RXDMAEN | SPI_CR2_TXDMAEN, 0);
+  } else {


davids5 · 2018-10-17T22:50:32Z

arch/arm/src/stm32f7/stm32_spi.c

@@ -263,8 +263,13 @@ static struct stm32_spidev_s g_spi1dev =
  .spiirq   = STM32_IRQ_SPI1,
 #endif
 #ifdef CONFIG_STM32F7_SPI_DMA
+#ifdef CONFIG_STM32F7_SPI1_DMA


Same as above

davids5 · 2018-10-17T22:50:47Z

arch/arm/src/stm32f7/stm32_spi.c

@@ -1704,25 +1736,31 @@ static void spi_bus_initialize(FAR struct stm32_spidev_s *priv)
   * priority inheritance enabled.
   */

-  sem_init(&priv->rxsem, 0, 0);
-  sem_init(&priv->txsem, 0, 0);
+  if (priv->rxch && priv->txch) {


dagar · 2018-10-18T15:01:37Z

Updated for coding standards and rebased on current px4_firmware_nuttx-7.22+.

Updated to coding standards.

davids5

@dagar Great! Thank you!

dagar · 2018-10-18T16:15:47Z

Upstream NuttX pull request. https://bitbucket.org/nuttx/nuttx/pull-requests/736/stm32-enable-separate-dma-per-spi/diff

dagar · 2018-10-18T19:46:53Z

Rebased on px4_firmware_nuttx-7.22+, PR now contains stm32f7 only.

davids5 · 2018-10-18T20:33:25Z

arch/arm/src/stm32f7/Kconfig

+config STM32F7_SPI1_DMA
+	bool "SPI1 DMA"
+	default n
+	depends on STM32F7_SPI_DMA


add the same chaed Greg did upstream

davids5 · 2018-10-19T15:13:24Z

Note to self: Vet F7 I2C driver, for same issues with ISR.

dagar · 2019-01-02T18:49:50Z

Upstream pull request for stm32f7. https://bitbucket.org/nuttx/nuttx/pull-requests/798/stm32f7-enable-separate-dma-per-spi/diff

dagar · 2019-01-02T18:52:22Z

Upstream pull request merged (https://bitbucket.org/nuttx/nuttx/commits/3b55f35bb32852c53a85d60c645da7b324f1508f). Given the timing of our NuttX upgrade I won't bother backporting.

* bbm: Fix errors * bbm: remove unnecessary code

Run all .c and .h files in last PR through nxstyle. Author: macman88 <jjlange91@gmail.com> SAME5x Ethernet Support (#25) boards/arm/samd5e5/same54-xplained-pro/: Adds basic support for Microchip SAM E54 Xplained Pro board. arch/arm/src/samd5e5/: Adds an Ethernet driver for the SAME5x family (based on the SAMA5 GMAC driver).

set CONFIG_PRIORITY_INHERITANCE=y set CONFIG_SEM_PREALLOCHOLDERS=0 or CONFIG_SEM_PREALLOCHOLDERS=8 #24 0x4dcab71 in __assert assert/lib_assert.c:37 #25 0x4d6b0e9 in nxsem_destroyholder semaphore/sem_holder.c:602 #26 0x4d80cf7 in nxsem_destroy semaphore/sem_destroy.c:80 #27 0x4d80db9 in sem_destroy semaphore/sem_destroy.c:120 #28 0x4dcb077 in nxmutex_destroy misc/lib_mutex.c:122 #29 0x4dc6611 in pipecommon_freedev pipes/pipe_common.c:117 #30 0x4dc7fdc in pipecommon_close pipes/pipe_common.c:397 #31 0x4ed4f6d in file_close vfs/fs_close.c:78 #32 0x6a91133 in local_free local/local_conn.c:184 #33 0x6a92a9c in local_release local/local_release.c:129 #34 0x6a91d1a in local_subref local/local_conn.c:271 #35 0x6a75767 in local_close local/local_sockif.c:797 #36 0x4e978f6 in psock_close socket/net_close.c:102 #37 0x4eed1b9 in sock_file_close socket/socket.c:115 #38 0x4ed4f6d in file_close vfs/fs_close.c:78 #39 0x4ed1459 in nx_close_from_tcb inode/fs_files.c:754 #40 0x4ed1501 in nx_close inode/fs_files.c:781 #41 0x4ed154a in close inode/fs_files.c:819 #42 0x6bcb9ce in property_get kvdb/client.c:307 #43 0x6bcd465 in property_get_int32 kvdb/common.c:270 #44 0x5106c9a in tz_offset_restore app/miwear_bluetooth.c:745 #45 0x510893f in miwear_bluetooth_main app/miwear_bluetooth.c:1033 #46 0x4dcf5c8 in nxtask_startup sched/task_startup.c:70 #47 0x4d70873 in nxtask_start task/task_start.c:134 #48 0x4e04a07 in pre_start sim/sim_initialstate.c:52 Signed-off-by: ligd <liguiding1@xiaomi.com>

dagar requested a review from davids5 July 9, 2018 22:04

dagar added the enhancement label Jul 9, 2018

dagar force-pushed the pr-stm32_spi_dma_bus branch from 51415bb to bca890d Compare July 9, 2018 22:13

davids5 reviewed Jul 9, 2018

View reviewed changes

LorenzMeier assigned davids5 Jul 10, 2018

dagar mentioned this pull request Jul 12, 2018

Design Roadmap: Driver Architecture, IMU Signal Processing, multi-EKF PX4/PX4-Autopilot#9891

Closed

dagar force-pushed the pr-stm32_spi_dma_bus branch 6 times, most recently from 64dfa70 to 8731ed1 Compare August 18, 2018 23:13

dagar force-pushed the pr-stm32_spi_dma_bus branch from 8731ed1 to ed45350 Compare September 11, 2018 16:59

dagar force-pushed the pr-stm32_spi_dma_bus branch from ed45350 to 4d49e84 Compare September 11, 2018 22:23

dagar force-pushed the pr-stm32_spi_dma_bus branch from 4d49e84 to 47f5bd9 Compare September 29, 2018 23:44

dagar requested a review from dakejahl October 17, 2018 15:01

davids5 previously requested changes Oct 17, 2018

View reviewed changes

dagar force-pushed the pr-stm32_spi_dma_bus branch from 47f5bd9 to c8d1c67 Compare October 18, 2018 15:01

davids5 previously approved these changes Oct 18, 2018

View reviewed changes

dagar mentioned this pull request Oct 18, 2018

[DO NOT MERGE] initial SPI DMA IMU driver (mpu9250) for testing/review/discussion PX4/PX4-Autopilot#10602

Closed

dagar mentioned this pull request Oct 18, 2018

[BACKPORT] stm32 enable separate DMA per SPI configuration #40

Merged

STM32F7 DMA per SPI

c808528

dagar dismissed davids5’s stale review via c808528 October 18, 2018 19:46

dagar force-pushed the pr-stm32_spi_dma_bus branch from c8d1c67 to c808528 Compare October 18, 2018 19:46

dagar changed the title ~~WIP: STM32 DMA per SPI~~ WIP: STM32F7 DMA per SPI Oct 18, 2018

davids5 reviewed Oct 18, 2018

View reviewed changes

dagar closed this Jan 2, 2019

dagar deleted the pr-stm32_spi_dma_bus branch January 2, 2019 18:52

Jun-Tao added a commit to Jun-Tao/nuttx that referenced this pull request Jul 4, 2019

bbm: Fix errors (PX4#25)

30053f2

* bbm: Fix errors * bbm: remove unnecessary code

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: STM32F7 DMA per SPI #25

WIP: STM32F7 DMA per SPI #25

dagar commented Jul 9, 2018 •

edited by AuterionWrikeBot

Loading

davids5 Jul 9, 2018

dagar Jul 10, 2018

davids5 Jul 10, 2018

davids5 Oct 17, 2018

davids5 Jul 9, 2018

dagar Jul 10, 2018

davids5 Jul 10, 2018 •

edited

Loading

LorenzMeier commented Jul 10, 2018

LorenzMeier commented Jul 10, 2018

dagar commented Aug 20, 2018

dagar commented Sep 11, 2018

thomasgubler commented Sep 24, 2018

dakejahl commented Oct 17, 2018

davids5 left a comment

davids5 Oct 17, 2018

davids5 Oct 17, 2018

davids5 Oct 17, 2018

davids5 Oct 17, 2018

davids5 Oct 17, 2018

davids5 Oct 17, 2018

dagar commented Oct 18, 2018

davids5 left a comment

dagar commented Oct 18, 2018 •

edited

Loading

dagar commented Oct 18, 2018

davids5 Oct 18, 2018

davids5 commented Oct 19, 2018

dagar commented Jan 2, 2019

dagar commented Jan 2, 2019


		spi_modifycr2(priv, SPI_CR2_RXDMAEN \| SPI_CR2_TXDMAEN, 0);
		} else {

WIP: STM32F7 DMA per SPI #25

WIP: STM32F7 DMA per SPI #25

Conversation

dagar commented Jul 9, 2018 • edited by AuterionWrikeBot Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davids5 Jul 10, 2018 • edited Loading

Choose a reason for hiding this comment

LorenzMeier commented Jul 10, 2018

LorenzMeier commented Jul 10, 2018

dagar commented Aug 20, 2018

dagar commented Sep 11, 2018

thomasgubler commented Sep 24, 2018

dakejahl commented Oct 17, 2018

davids5 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dagar commented Oct 18, 2018

davids5 left a comment

Choose a reason for hiding this comment

dagar commented Oct 18, 2018 • edited Loading

dagar commented Oct 18, 2018

Choose a reason for hiding this comment

davids5 commented Oct 19, 2018

dagar commented Jan 2, 2019

dagar commented Jan 2, 2019

dagar commented Jul 9, 2018 •

edited by AuterionWrikeBot

Loading

davids5 Jul 10, 2018 •

edited

Loading

dagar commented Oct 18, 2018 •

edited

Loading