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/10 06:02:37 UTC

[GitHub] [incubator-nuttx] hartmannathan opened a new pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   ## Summary
   
   arch/arm/src/stm32/hardware/stm32_spi.h:
   
   - Handle register variations for STM32G47xxx.
   - Handle STM32G47xxx in conditional logic.
   - Make conditional logic for I2S and TI frame format mode more self-documenting by creating HAVE_SPI_I2S and HAVE_SPI_TI_MODE defines.
   - Fix nxstyle errors.
   
   ## Impact
   
   Allows STM32G47xxx MCUs to gain SPI support.
   
   ## Testing
   
   nxstyle.


----------------------------------------------------------------
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 #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   @hartmannathan  Looking good! how about a squash and force push before we review 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] davids5 commented on a change in pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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



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

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


   @davids5 Ok it's squashed and force pushed... Give it one last lookover and if it looks good, do the honors!


----------------------------------------------------------------
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] hartmannathan commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   On Wed, Nov 11, 2020 at 7:27 AM David Sidrane <no...@github.com>
   wrote:
   
   > *@davids5* requested changes on this pull request.
   >
   > What is value in having this in Kconfig?
   > ------------------------------
   >
   > In arch/arm/src/stm32/Kconfig
   > <https://github.com/apache/incubator-nuttx/pull/2273#discussion_r521322607>
   > :
   >
   > > @@ -2417,6 +2417,22 @@ config STM32_HAVE_IP_DMA_V2
   >  	bool
   >  	default n
   >
   > +config STM32_HAVE_IP_SPI_V1
   >
   > These are data sheet dependent and invariant. Why not encapsulate it in
   > the chip.h? Leaving it up the the end user requires them to do work that
   > was already done when an new SoC is added.
   >
   
   The end user does not have to do anything. For all the STM32 chips,
   peripheral IP versions are selected in Kconfig. See the similar
   automatically-selected configs for other peripherals' versions. Putting
   this one in chip.h would be a departure from how it is already being done
   for all other peripherals.
   


----------------------------------------------------------------
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] hartmannathan commented on a change in pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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



##########
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:
       > The idea is a single line ifdef
   > 
   > `#if defined(STM32_HAS_SPI_CRC))`
   > 
   > Then in the chip file define `#define STM32_HAS_SPI_CRC ` For all the chips that support it.
   
   On 2nd thought, are you sure the HAVE_* macros should go in chip.h? Looking at that file, it contains only number-of-peripherals macros like STM32_NGPIO, STM32_NDAC, etc. No HAVE_* macros for anything.
   
   I have made the changes in stm32_spi.h and have not moved any HAVE_* macros to chip.h yet. Please see if this is acceptable. Thanks




----------------------------------------------------------------
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 #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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



##########
File path: arch/arm/src/stm32/Kconfig
##########
@@ -2417,6 +2417,22 @@ config STM32_HAVE_IP_DMA_V2
 	bool
 	default n
 
+config STM32_HAVE_IP_SPI_V1

Review comment:
       These are data sheet dependent and invariant. Why not encapsulate it in the chip.h?  Leaving it up the the end user requires them to do work that was already done when an new SoC is added.  




----------------------------------------------------------------
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] raiden00pl commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   I don't think we can move all IP version configurations to chip.h. Peripherals functionality depends on the IP version and this information is needed in Kconfig (eg. the number of PWM channels or ADC DMA mode). If we can't move everything to chip.h, does that still make sense ?


----------------------------------------------------------------
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] hartmannathan commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   @davids5 @raiden00pl I have the pre-squashed version where I was setting the IP version in Kconfig. If it's better to reinstate that code, let me know and I'll open a separate PR.


----------------------------------------------------------------
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] raiden00pl commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   I agree that the main problem with the current solution is consistency. But the next problem I see is less flexibility. 
   If in the future ST introduces some new functionality to SPI which must be configurable in Kconfig, then we have a problem.
   Therefore, I believe that it is better to keep all IP versions in Kconfig, even it it is not a perfect solution.


----------------------------------------------------------------
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 #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   @hartmannathan - Do you envision the SPI IP version will enable a user choice? As the roll up of CONFIG_HAVE_TIMER5 into STM32_HAVE_IP_TIMERS_V2 does?


----------------------------------------------------------------
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] raiden00pl commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   > There other is invariant it is the version of IP on the die. The users choice does can not change the IP version in the the chip.
   
   You have to know the IP version first and then expose the available resources to the user in Kconfig. If Kconfig doesn't know the IP version, it cannot show the user the available options.
   
   eg.
   
   ```
   config STM32_TIM1_CHANNEL5
   	bool "TIM1 Channel 5 (internal)"
   	default n
   	depends on STM32_HAVE_IP_TIMERS_V2
   	---help---
   		Enables channel 5 (no available externaly)
   
   ```


----------------------------------------------------------------
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] hartmannathan commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   > @hartmannathan Looking good! how about a squash and force push before we review it?
   
   Yes. As soon as the tests finish I'll squash 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] davids5 commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   @raiden00pl - Got it. Thank you for the example.   I think the register case is different.  


----------------------------------------------------------------
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] hartmannathan commented on a change in pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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



##########
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:
       On further study it seems that for STM32, the STM32_HAVE_IP_* are setup in Kconfig, for example, STM32_HAVE_IP_TIMERS_V1 and others. These are selected in nuttx/arch/arm/src/stm32/Kconfig based on the chip model.
   
   Since this is the precedent for STM32, this PR should follow it.
   
   If I roll it up and label it as v1, v2, v3, I'm trying to figure out which features are in which version...




----------------------------------------------------------------
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 #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   > I don't think we can move all IP version configurations to chip.h. Peripherals functionality depends on the IP version and this information is needed in Kconfig (eg. the number of PWM channels or ADC DMA mode). If we can't move everything to chip.h, does that still make sense ?
   
   I think so. One is a user choice for configuration. I have a board that use 2 PWM or I want to use the ADC DMA in XXX mode.  
   
   There other is invariant it is the version of IP on the die. The users choice does can not change the IP version in the the chip. 
   
   


----------------------------------------------------------------
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 #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   @raiden00pl - theses are all good arguments.  On the consistency aspect: Shall we do away with chip.h and move all the information to Kconfig? (remember that Kconfig was added later in the nuttix time line.) 


----------------------------------------------------------------
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 #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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



##########
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))`
   
   Then 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



[GitHub] [incubator-nuttx] davids5 merged pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   


----------------------------------------------------------------
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 #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   > putting this one in chip.h would be a departure from how it is already being done
   for all other peripherals.
   
   Just like removing all the ifdef rash, departures form incorrect are a good thing. Putting invariant in Kconfig is not a proper use for it. The Chip file describes the number and version of IP. It is invariant.  The IP on a chip does not change. 
   
   It makes sense for the Device subfamily,  Flash memory size, and Package etc. 
   ![image](https://user-images.githubusercontent.com/1945821/98817194-9ede1b00-23de-11eb-962e-06b6aa13b175.png)
   
   It does not make sense for the IP on the die. 
   


----------------------------------------------------------------
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 #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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



##########
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:
       Look how it is done in Kinetis.  https://github.com/apache/incubator-nuttx/blob/master/arch/arm/include/kinetis/kinetis_mcg.h#L50 
   
   That is the knot that pulls all the IP blocks together for the mgc. If you go up one level the other knots sim and pmc are there as well.  
   
   We can do something like that for the STM32 IP blocks. 
   
   Chip.h is the "differentiator" listing N of IP blocks. It would make sense to silo the version of IP the same way.  The SoC (chip) "has-a" set of IP that is as some revision. It will be used in other SoC. If you can name the feature then add the feature to the chip section in chip file. That is great. If you can roll it it up and label it.  STM32_SPI_Vn that that will work well to. 




----------------------------------------------------------------
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] hartmannathan commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   > @hartmannathan - Do you envision the SPI IP version will enable a user choice? As the roll up of CONFIG_HAVE_TIMER5 into STM32_HAVE_IP_TIMERS_V2 does?
   
   The only one I can think of is whether you want to use the SPI block as I2S. It has a mode like that in the newer IP versions. I don't know if we even handle or support that.
   
   The larger concern for me is consistency and maintenance. I'd rather avoid having some IP versions defined in one location and others defined in another location, if feasible to keep the logic in one place.
   
   But I don't have a strong opinion. I'm okay with it as it is now, or back in Kconfig. Either option is fine by me.


----------------------------------------------------------------
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] hartmannathan commented on a change in pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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



##########
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:
       Yes. I am trying to reduce the ifdef rash in this file. Already added HAVE_SPI_I2S and HAVE_SPI_TI_MODE. I'll add a STM32_HAS_SPI_CRC as well. Makes sense to put it in chip.h file.




----------------------------------------------------------------
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] hartmannathan commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   > > putting this one in chip.h would be a departure from how it is already being done
   > > for all other peripherals.
   > 
   > Just like removing all the ifdef rash, departures form incorrect are a good thing. Putting invariant in Kconfig is not a proper use for it. The Chip file describes the number and version of IP. It is invariant. The IP on a chip does not change.
   
   Fair enough. Since this PR is for the stm32-spi.h file, I will move the SPI IP version from Kconfig to chip.h. Those for other peripherals will need to be refactored in separate PRs.


----------------------------------------------------------------
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] raiden00pl commented on pull request #2273: stm32/stm32_spi: Add SPI register definitions for STM32G47xxx

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


   @davids5 It could be good. I think we already have almost all the necessary information in Kconfig, directly or indirectly. 
   The only problem here is the STM32_NGPIO definitions. Moving these to Kconfig will add a lot of new lines to a file that is already quite large.


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