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/09/08 20:10:09 UTC

[GitHub] [incubator-nuttx] davids5 opened a new pull request #1734: Allow dma capable to be permissive

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


   ## Summary
     A driver passing a buffer that is aligned and sized on d-cache bounders but does not used the full extent of the buffer will fail the dma_capable test and in a debug build DEBUGASSERT. 
   
   
   This PR adds the option configures the stm32_dmacapable to not disqualify DMA operations on memory that is not dcache aligned based solely on the starting addresss and byte count.
   
   One would use this when ALL buffer extents are known to be aligned, but the the count does not use the complete buffer.
   
   ## Impact
     None - if disabled and it is by default.
   
   ## Testing
   
   Run a debug build and pass a buffer that is 64 bytes and aligned on 32, but a count of 16 bytes to DMA. It will no longer assert.
   


----------------------------------------------------------------
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] jerpelea commented on pull request #1734: Allow dma capable to be permissive

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


   LGTM please fix CI error


----------------------------------------------------------------
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 #1734: Allow dma capable to be permissive

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


   > LGTM please fix CI error
   
   As I said above: please ignore (or suggest a remedy) for the alignment of the braces. It looks fine to me, but the `ifdef` must be screwing with the detection.
   
   Hi @jerpelea - any suggestions on how to fix it?


----------------------------------------------------------------
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] jerpelea merged pull request #1734: Allow dma capable to be permissive

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


   


----------------------------------------------------------------
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] ghn-certi commented on a change in pull request #1734: Allow dma capable to be permissive

Posted by GitBox <gi...@apache.org>.
ghn-certi commented on a change in pull request #1734:
URL: https://github.com/apache/incubator-nuttx/pull/1734#discussion_r485231824



##########
File path: arch/arm/src/stm32h7/Kconfig
##########
@@ -1684,6 +1684,16 @@ config STM32H7_DMACAPABLE
 		Drivers then may use this information to determine if they should
 		attempt the DMA or fall back to a different transfer method.
 
+config STM32H7_DMACAPABLE_ASSUME_CACHE_ALIGNED
+	bool "Do not disqualify DMA capability based on cache alignment"
+	depends on STM32H7_DMACAPABLE && ARMV7M_DCACHE && !ARMV7M_DCACHE_WRITETHROUGH
+	default n
+	---help---
+		This option configures the stm32_dmacapable to not disqualify
+		DMA operations on memory that is not dcache aligned based solely
+		on the starting addresss and byte count.
+		Use this when ALL buffer extents are known to be alligned, but the

Review comment:
       ```suggestion
   		Use this when ALL buffer extents are known to be aligned, but the
   ```




----------------------------------------------------------------
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 #1734: Allow dma capable to be permissive

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



##########
File path: arch/arm/src/stm32h7/Kconfig
##########
@@ -1684,6 +1684,16 @@ config STM32H7_DMACAPABLE
 		Drivers then may use this information to determine if they should
 		attempt the DMA or fall back to a different transfer method.
 
+config STM32H7_DMACAPABLE_ASSUME_CACHE_ALIGNED
+	bool "Do not disqualify DMA capability based on cache alignment"
+	depends on STM32H7_DMACAPABLE && ARMV7M_DCACHE && !ARMV7M_DCACHE_WRITETHROUGH
+	default n
+	---help---
+		This option configures the stm32_dmacapable to not disqualify
+		DMA operations on memory that is not dcache aligned based solely
+		on the starting addresss and byte count.
+		Use this when ALL buffer extents are known to be alligned, but the

Review comment:
       @ghn-certi - Thank you for the review. Fixed and force pushed.
   




----------------------------------------------------------------
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] jerpelea commented on pull request #1734: Allow dma capable to be permissive

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


   we have 2 options 
   - add 4 spaces instead of 2 before all brackets and we avoid future errors
   - ignore the error


----------------------------------------------------------------
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 pull request #1734: Allow dma capable to be permissive

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


   > @davids5 we have 2 options
   > 
   >     * add 4 spaces instead of 2 before all brackets and we avoid future errors
   > 
   >     * ignore the error
   
   I'd suggest we ignore it for this PR and address the nxstyle issue later.  The alignment seems fine and we have a lot of similar cases in the code base.


----------------------------------------------------------------
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 #1734: Allow dma capable to be permissive

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


   @jerpelea @Ouss4 - Agreed! - Who would like to push the merge button?


----------------------------------------------------------------
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 #1734: Allow dma capable to be permissive

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


   Please ignore (or suggest a remedy) for the alignment of the braces. It looks fine to me, but the ifdef must be screwing with the detection.


----------------------------------------------------------------
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] jerpelea edited a comment on pull request #1734: Allow dma capable to be permissive

Posted by GitBox <gi...@apache.org>.
jerpelea edited a comment on pull request #1734:
URL: https://github.com/apache/incubator-nuttx/pull/1734#issuecomment-689477547


   @davids5 we have 2 options 
   - add 4 spaces instead of 2 before all brackets and we avoid future errors
   - ignore the error


----------------------------------------------------------------
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] jerpelea commented on pull request #1734: Allow dma capable to be permissive

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


   > 
   > 
   > > @davids5 we have 2 options
   > > ```
   > > * add 4 spaces instead of 2 before all brackets and we avoid future errors
   > > 
   > > * ignore the error
   > > ```
   > 
   > I'd suggest we ignore it for this PR and address the nxstyle issue later. The alignment seems fine and we have a lot of similar cases in the code base.
   
   +1


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