[GitHub] [incubator-nuttx] anpaza commented on a change in pull request #459: stm32h7: support SDRAM via FMC peripherial

[GitBox <gi...@apache.org>]

anpaza commented on a change in pull request #459: stm32h7: support SDRAM via FMC peripherial
URL: https://github.com/apache/incubator-nuttx/pull/459#discussion_r388998837
 
 

 ##########
 File path: arch/arm/src/common/up_arch.h
 ##########
 @@ -58,6 +58,12 @@
 # define getreg32(a)          (*(volatile uint32_t *)(a))
 # define putreg32(v,a)        (*(volatile uint32_t *)(a) = (v))
 
+/* Non-atomic, but more effective modification of registers */
+
+# define modreg8(v,m,a)       putreg8((getreg8(a) & ~(m)) | ((v) & (m)), a)
+# define modreg16(v,m,a)      putreg16((getreg16(a) & ~(m)) | ((v) & (m)), a)
+# define modreg32(v,m,a)      putreg32((getreg32(a) & ~(m)) | ((v) & (m)), a)
+
 
 Review comment:
   They are used in stm32_fmc.c (modreg32).
   
   I think it is easier to read this:
   
   modreg32 (BOARD_FMC_CLK, RCC_D1CCIPR_FMCSEL_MASK, STM32_RCC_D1CCIPR);
   
   than the old-style equivalent:
   
     regval  = getreg32(STM32_RCC_D1CCIPR);
     regval = (regval & ~RCC_D1CCIPR_FMCSEL_MASK) | BOARD_FMC_CLK;
     putreg32(regval, STM32_RCC_D1CCIPR);
   
   Also I have pending changes to e.g. stm32_ethernet.c using these macros.
   
   I noted that there's a new family of functions modifyreg*() (that were not available on nuttx-8.2 I was working on), but they introduce extra cost for every call, and won't protect  anything in my case (and many other cases, as usually modifications to several registers have to be atomic, not individual register modifications).

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