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 2021/04/28 08:10:02 UTC
[GitHub] [incubator-nuttx] AlexanderVasiljev opened a new pull request #3618: stm32h7: serial: use dma tx semaphore as resource holder
AlexanderVasiljev opened a new pull request #3618:
URL: https://github.com/apache/incubator-nuttx/pull/3618
## Summary
This commit will correct error in the stm32H7 serial dma transmition.
txdmasem semaphore is posted every time in up_dma_txcallback. But it is waited occasionally in up_dma_txavailable. So its value is always positive and it never falls asleep.
This commit will use semaphore as resource holder
--
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] AlexanderVasiljev commented on a change in pull request #3618: stm32h7: serial: use dma tx semaphore as resource holder
Posted by GitBox <gi...@apache.org>.
AlexanderVasiljev commented on a change in pull request #3618:
URL: https://github.com/apache/incubator-nuttx/pull/3618#discussion_r622245636
##########
File path: arch/arm/src/stm32h7/stm32_serial.c
##########
@@ -3375,10 +3375,7 @@ static void up_dma_txavailable(struct uart_dev_s *dev)
/* Only send when the DMA is idle */
- if (resid != 0)
Review comment:
> I think this is the wrong approach. We have to wait if there is a residual.
Why we should care about residuals?
Every **up_dma_txavailable** should be followed by one **up_dma_txcallback**. After **up_dma_txcallback** residuals should be zero. This all we need.
> Having an initial count of 1 means the first time, the DMA can be active and we ignore it.
The DMA shouldn't be activated before any **up_dma_txavailable**. So the first **up_dma_txavailable** takes the resource named DMA transmit instance. **up_dma_txcallback** will free resource. Or is there any other who can trigger the DMA transmit?
> I think the correct approach it the DMA completion should check the count and only post if the semaphore is being waited on.
Manually checking a semaphore count has never been a good thing. You will definitely fall into the race condition. You can check the count, but on the next moment everything can turn upside down.
--
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 #3618: stm32h7: serial: use dma tx semaphore as resource holder
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #3618:
URL: https://github.com/apache/incubator-nuttx/pull/3618#issuecomment-829079291
@AlexanderVasiljev - Thank you! I will merge this once CI finishes.
--
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] AlexanderVasiljev commented on a change in pull request #3618: stm32h7: serial: use dma tx semaphore as resource holder
Posted by GitBox <gi...@apache.org>.
AlexanderVasiljev commented on a change in pull request #3618:
URL: https://github.com/apache/incubator-nuttx/pull/3618#discussion_r622299144
##########
File path: arch/arm/src/stm32h7/stm32_serial.c
##########
@@ -3375,10 +3375,7 @@ static void up_dma_txavailable(struct uart_dev_s *dev)
/* Only send when the DMA is idle */
- if (resid != 0)
Review comment:
Yes, it is redundant.
--
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 a change in pull request #3618: stm32h7: serial: use dma tx semaphore as resource holder
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #3618:
URL: https://github.com/apache/incubator-nuttx/pull/3618#discussion_r622175784
##########
File path: arch/arm/src/stm32h7/stm32_serial.c
##########
@@ -3375,10 +3375,7 @@ static void up_dma_txavailable(struct uart_dev_s *dev)
/* Only send when the DMA is idle */
- if (resid != 0)
Review comment:
I think this is the wrong approach. We have to wait if there is a residual.
Having an initial count of 1 means the first time, the DMA can be active and we ignore it.
I think the correct approach it the DMA completion should check the count and only post if the semaphore is being waited on.
--
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 #3618: stm32h7: serial: use dma tx semaphore as resource holder
Posted by GitBox <gi...@apache.org>.
davids5 merged pull request #3618:
URL: https://github.com/apache/incubator-nuttx/pull/3618
--
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 a change in pull request #3618: stm32h7: serial: use dma tx semaphore as resource holder
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #3618:
URL: https://github.com/apache/incubator-nuttx/pull/3618#discussion_r622293887
##########
File path: arch/arm/src/stm32h7/stm32_serial.c
##########
@@ -3375,10 +3375,7 @@ static void up_dma_txavailable(struct uart_dev_s *dev)
/* Only send when the DMA is idle */
- if (resid != 0)
Review comment:
Ok then how about removing the residue code?
--
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