[GitHub] [incubator-nuttx] davids5 commented on a change in pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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

davids5 commented on a change in pull request #2273:
URL: https://github.com/apache/incubator-nuttx/pull/2273#discussion_r520596645



##########
File path: arch/arm/src/stm32/hardware/stm32_spi.h
##########
@@ -47,6 +47,24 @@
  * Pre-processor Definitions
  ************************************************************************************/
 
+/* SPI version **********************************************************************/

Review comment:
       Should these go in the chip.h file?

##########
File path: arch/arm/src/stm32/hardware/stm32_spi.h
##########
@@ -135,7 +150,8 @@
 #define SPI_CR1_SSI               (1 << 8)  /* Bit 8: Internal slave select */
 #define SPI_CR1_SSM               (1 << 9)  /* Bit 9: Software slave management */
 #define SPI_CR1_RXONLY            (1 << 10) /* Bit 10: Receive only */
-#if defined(CONFIG_STM32_STM32F30XX) || defined(CONFIG_STM32_STM32F37XX)
+#if defined(CONFIG_STM32_STM32F30XX) || defined(CONFIG_STM32_STM32F37XX) \

Review comment:
       Should we not increase the ifdef rash? 
   
   Using HAS_xxxxx macros and distribute them to the chip file?
   
   The idea is a single line ifdef
   
   `#if defined(STM32_HAS_SPI_CRC))`
   
   This in the chip file define `#define STM32_HAS_SPI_CRC ` For all the chips that support 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