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/11/17 11:46:42 UTC
[GitHub] [incubator-nuttx] juniskane opened a new pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
juniskane opened a new pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325
arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom for enabling/disabling RX interrupts
## Summary
Code cleanup. Change makes it easier to diff serial driver between various stm32*
subarchitectures. stm32f7, stm32h7, stm32l4 and stm32f0l0g0 do it this way and there is no
reason for classic stm32 to differ.
Also manipulation of priv->ie was not atomic with respect to interrupts.
## Impact
Should be no functional changes
## Testing
Tested with proprietary STM32L1 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] davids5 commented on pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325#issuecomment-728889446
@juniskane - I think there is a subtle changes. Ins't the The DMA disable path is now different? Did you test with DMA & flow control?
----------------------------------------------------------------
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] davids5 merged pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325
----------------------------------------------------------------
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] juniskane commented on pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
Posted by GitBox <gi...@apache.org>.
juniskane commented on pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325#issuecomment-728950144
@davids5 : unfortunately not really tested DMA + flow control with upstream NuttX because for that STM32L1 needs the functionality reverted by df9ae3c13f. It would be great if someone could test this with F3 and F4 (I have no board). But isn't the correct thing here to enable/disable the DMA, as uart_enablerxint() and uart_disablerxint() helper macros do, not the interrupt?
The original code cannot be deemed correct either, as it does not even compile when SERIAL_HAVE_ONLY_DMA is true, as up_rxint() is not defined then.
----------------------------------------------------------------
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] juniskane commented on pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
Posted by GitBox <gi...@apache.org>.
juniskane commented on pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325#issuecomment-731595332
> @juniskane - I would like to test this. Unfortunately I can not spend time on this in the latter half of next week. Will that be OK?
Sure, take your time. The "classic" STM32 boards are just hobby stuff for me anyhow :) I think this has been broken since time being, not a new regression. I'll leave this pull request open for now, but lets close it without merging if no progress. I think company has some spare F4 boards at office, I'll try to acquire one for testing serial on that arch sometime during next week.
----------------------------------------------------------------
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] btashton commented on pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
Posted by GitBox <gi...@apache.org>.
btashton commented on pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325#issuecomment-732385486
Should we include this in the 10.0.0 release? If so @davids5 can you just add the label and it will go into the queue
----------------------------------------------------------------
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] juniskane edited a comment on pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
Posted by GitBox <gi...@apache.org>.
juniskane edited a comment on pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325#issuecomment-731592058
Tried to verify this with stock STM32L152C-Discovery board. STM32L1 serial with RX DMA is broken on this arch even without any flowcontrol (without further Haltian patches that have been rejected by upstream before), so unable to verify. I tried with this delta to stm32ldiscovery:nsh config:
```
--- .config.nsh.orig 2020-11-21 16:51:36.015255226 +0200
+++ .config 2020-11-21 16:57:02.622910842 +0200
@@ -530,8 +530,8 @@
# CONFIG_STM32_ADC2 is not set
# CONFIG_STM32_CAN1 is not set
# CONFIG_STM32_CRC is not set
-# CONFIG_STM32_DMA1 is not set
-# CONFIG_STM32_DMA2 is not set
+CONFIG_STM32_DMA1=y
+CONFIG_STM32_DMA2=y
# CONFIG_STM32_DAC1 is not set
# CONFIG_STM32_HRTIM is not set
# CONFIG_STM32_I2C1 is not set
@@ -575,8 +575,9 @@
# CONFIG_STM32_DISABLE_IDLE_SLEEP_DURING_DEBUG is not set
# CONFIG_STM32_FORCEPOWER is not set
# CONFIG_ARCH_BOARD_STM32_CUSTOM_CLOCKCONFIG is not set
+# CONFIG_STM32_DMACAPABLE is not set
CONFIG_STM32_USART=y
-# CONFIG_STM32_USART_RXDMA is not set
+CONFIG_STM32_USART_RXDMA=y
CONFIG_STM32_SERIALDRIVER=y
# CONFIG_STM32_1WIREDRIVER is not set
# CONFIG_STM32_HCIUART is not set
@@ -592,12 +593,13 @@
CONFIG_STM32_USART1_SERIALDRIVER=y
# CONFIG_STM32_USART1_1WIREDRIVER is not set
# CONFIG_USART1_RS485 is not set
-# CONFIG_USART1_RXDMA is not set
+CONFIG_USART1_RXDMA=y
# CONFIG_USART1_TXDMA is not set
#
# Serial Driver Configuration
#
+CONFIG_STM32_SERIAL_RXDMA_BUFFER_SIZE=32
# CONFIG_STM32_SERIAL_DISABLE_REORDERING is not set
# CONFIG_STM32_FLOWCONTROL_BROKEN is not set
# CONFIG_STM32_USART_BREAKS is not set
@@ -626,7 +628,7 @@
# CONFIG_ARCH_NOINTC is not set
# CONFIG_ARCH_VECNOTIRQ is not set
CONFIG_ARCH_HAVE_IRQTRIGGER=y
-# CONFIG_ARCH_DMA is not set
+CONFIG_ARCH_DMA=y
CONFIG_ARCH_HAVE_IRQPRIO=y
# CONFIG_ARCH_ICACHE is not set
# CONFIG_ARCH_DCACHE is not set
@@ -940,7 +942,7 @@
# CONFIG_SERIAL_RS485CONTROL is not set
# CONFIG_SERIAL_OFLOWCONTROL is not set
# CONFIG_SERIAL_TXDMA is not set
-# CONFIG_SERIAL_RXDMA is not set
+CONFIG_SERIAL_RXDMA=y
# CONFIG_SERIAL_TERMIOS is not set
CONFIG_USART1_SERIAL_CONSOLE=y
# CONFIG_OTHER_SERIAL_CONSOLE is not set
```
I don't have F2/F3/F4 board to test with. Giving up, as not worthwhile to spent too much time with obsolete architecture.
@davids5: Do you have better idea, how to fix the build error with CONFIG_SERIAL_IFLOWCONTROL && CONFIG_SERIAL_RXDMA on this variant of stm32_serial.c, when all configured USART use RX DMA? Just document somewhere that this is broken? The error was:
```
/home/jni/osdev/nuttx_github/nuttx/staging/libarch.a(stm32_serial.o): In function `up_rxflowcontrol':
stm32_serial.c:(.text+0xe4): undefined reference to `up_rxint'
Makefile:171: recipe for target 'nuttx' failed
```
----------------------------------------------------------------
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] juniskane commented on pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
Posted by GitBox <gi...@apache.org>.
juniskane commented on pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325#issuecomment-731592058
Tried to verify this with stock STM32L152C-Discovery board. STM32L1 serial with RX DMA is broken on this arch even without any flowcontrol (without further Haltian patches that have been rejected by upstream before), so unable to verify. I tried with this delta to stm32ldiscovery:nsh config:
```
--- .config.nsh.orig 2020-11-21 16:51:36.015255226 +0200
+++ .config 2020-11-21 16:57:02.622910842 +0200
@@ -530,8 +530,8 @@
# CONFIG_STM32_ADC2 is not set
# CONFIG_STM32_CAN1 is not set
# CONFIG_STM32_CRC is not set
-# CONFIG_STM32_DMA1 is not set
-# CONFIG_STM32_DMA2 is not set
+CONFIG_STM32_DMA1=y
+CONFIG_STM32_DMA2=y
# CONFIG_STM32_DAC1 is not set
# CONFIG_STM32_HRTIM is not set
# CONFIG_STM32_I2C1 is not set
@@ -575,8 +575,9 @@
# CONFIG_STM32_DISABLE_IDLE_SLEEP_DURING_DEBUG is not set
# CONFIG_STM32_FORCEPOWER is not set
# CONFIG_ARCH_BOARD_STM32_CUSTOM_CLOCKCONFIG is not set
+# CONFIG_STM32_DMACAPABLE is not set
CONFIG_STM32_USART=y
-# CONFIG_STM32_USART_RXDMA is not set
+CONFIG_STM32_USART_RXDMA=y
CONFIG_STM32_SERIALDRIVER=y
# CONFIG_STM32_1WIREDRIVER is not set
# CONFIG_STM32_HCIUART is not set
@@ -592,12 +593,13 @@
CONFIG_STM32_USART1_SERIALDRIVER=y
# CONFIG_STM32_USART1_1WIREDRIVER is not set
# CONFIG_USART1_RS485 is not set
-# CONFIG_USART1_RXDMA is not set
+CONFIG_USART1_RXDMA=y
# CONFIG_USART1_TXDMA is not set
#
# Serial Driver Configuration
#
+CONFIG_STM32_SERIAL_RXDMA_BUFFER_SIZE=32
# CONFIG_STM32_SERIAL_DISABLE_REORDERING is not set
# CONFIG_STM32_FLOWCONTROL_BROKEN is not set
# CONFIG_STM32_USART_BREAKS is not set
@@ -626,7 +628,7 @@
# CONFIG_ARCH_NOINTC is not set
# CONFIG_ARCH_VECNOTIRQ is not set
CONFIG_ARCH_HAVE_IRQTRIGGER=y
-# CONFIG_ARCH_DMA is not set
+CONFIG_ARCH_DMA=y
CONFIG_ARCH_HAVE_IRQPRIO=y
# CONFIG_ARCH_ICACHE is not set
# CONFIG_ARCH_DCACHE is not set
@@ -940,7 +942,7 @@
# CONFIG_SERIAL_RS485CONTROL is not set
# CONFIG_SERIAL_OFLOWCONTROL is not set
# CONFIG_SERIAL_TXDMA is not set
-# CONFIG_SERIAL_RXDMA is not set
+CONFIG_SERIAL_RXDMA=y
# CONFIG_SERIAL_TERMIOS is not set
CONFIG_USART1_SERIAL_CONSOLE=y
# CONFIG_OTHER_SERIAL_CONSOLE is not set
```
I don't have F2/F3/F4 board to test with. Giving up, as not worthwhile to spent too much time with obsolete architecture.
@davids5: Do you have better idea, how to fix the build error with CONFIG_SERIAL_IFLOWCONTROL && CONFIG_SERIAL_RXDMA on this variant of stm32_serial.c, when all configured USART use RX DMA? Just document somewhere that this is broken?
----------------------------------------------------------------
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] acassis commented on pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
Posted by GitBox <gi...@apache.org>.
acassis commented on pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325#issuecomment-731140386
Hi @juniskane there is a flow control test application at apps/examples/flowc I think it could be used to validate this patch. Did you use it or only tested communicating with some external device (i.e. GSM Modem) ?
----------------------------------------------------------------
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] davids5 commented on pull request #2325: arch/arm/src/stm32/stm32_serial.c: for flowcontrol use common idiom f…
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #2325:
URL: https://github.com/apache/incubator-nuttx/pull/2325#issuecomment-731592433
@juniskane - I would like to test this. Unfortunately I can not spend time on this in the latter half of next week. Will that be 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org