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 2022/07/21 00:49:41 UTC

[GitHub] [incubator-nuttx] curuvar opened a new pull request, #6648: Added Adafruit QT Py RP2040 board and made UART, SPI, and I2C configuration more flexible.

curuvar opened a new pull request, #6648:
URL: https://github.com/apache/incubator-nuttx/pull/6648

   
   ## Summary
   
   * Added Adafruit QT Py RP2040 board. 
   
   * Added ability to configure individual UART, SPI and I2C pin locations.  
   
   The current RP2040 configuration code assumed all pins for a give function were contiguous. For example, in the configuration you could set the UART0 TX pin to GPIO0. This would automatically set the UART0 RX to GPIO1, CTS to GPIO2, and RTS to GPIO3, with no opportunity to change this.  The new configuration let you independendly set all four to any supported pin.
   
   This change was made to the UARTs, I2Cs, and SPIs.  As part of the is change the UART, I2C, and SPI configuration was moved from the individual board's Kconfig to the RP2040 architecture Kconfig.  This eliminates a lot of duplication and seemed appropriate as the pin assignments are more of an RP2040 thing than an individual board thing.
   
   The ability to configure pins independently is necessary for the QT Py as its labeled TX pin is GPIO20 and its TX pin is GPIO5.  You could not configure NuttX this way without allowing pins to be configured independently.
   
   ## Impact
   
   Any existing project will need to have its .config file updated to include the new configuration options. I tried to set reasonable defaults to match most of the prebuilt configurations.
   
   ## Testing
   
   I have built NuttX with all the various UART, I2C, and SPI selections enabled, and tested various UART and SPI slave combinations to make sure they still worked. 
   
   


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

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #6648: Added Adafruit QT Py RP2040 board and made UART, SPI, and I2C configuration more flexible.

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on code in PR #6648:
URL: https://github.com/apache/incubator-nuttx/pull/6648#discussion_r926663557


##########
arch/arm/src/rp2040/Kconfig:
##########
@@ -13,9 +20,63 @@ config RP2040_UART0
 
 if RP2040_UART0
 
+config RP2040_UART0_TX_GPIO

Review Comment:
   Usually peripheral driver options are out of peripheral selection menu. For example if I select `SPI0` peripheral for the "arch" I need to go to another menu to select polling vs DMA or enable other arch SPI options.
   The pins are the board level configuration and not the "arch" level configuration and usually are defined in board level header file. I understand that for the boards that have many pins exposed to connectors it is reasonable to have configuration options for those pins, but I think that grouping pin configuration at board level makes more sense. At least if config of all pins are on the same screen the visual check may be done to inspect if there are not duplicates.
   So my opinion is to have such config at board level 



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

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] curuvar commented on a diff in pull request #6648: Added Adafruit QT Py RP2040 board and made UART, SPI, and I2C configuration more flexible.

Posted by GitBox <gi...@apache.org>.
curuvar commented on code in PR #6648:
URL: https://github.com/apache/incubator-nuttx/pull/6648#discussion_r926545372


##########
arch/arm/src/rp2040/Kconfig:
##########
@@ -13,9 +20,63 @@ config RP2040_UART0
 
 if RP2040_UART0
 
+config RP2040_UART0_TX_GPIO

Review Comment:
   I'm open to doing this.  
   
   Currently a user must go the the "arch" options to turn on UART0, but then go to a completely different menu to configure the UART.  This seems wrong to me.  My goal was to keep all the pin configuration options in the same place within the menuconfig structure and since the pins are a feature of the RP2040 chip itself, it seemed better to do that in "arch" rather than "board".
   
   If you think that all these configuration options would be better placed under the "board" menu, I'll move them all there.



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

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] curuvar commented on a diff in pull request #6648: Added Adafruit QT Py RP2040 board and made UART, SPI, and I2C configuration more flexible.

Posted by GitBox <gi...@apache.org>.
curuvar commented on code in PR #6648:
URL: https://github.com/apache/incubator-nuttx/pull/6648#discussion_r926851653


##########
arch/arm/src/rp2040/Kconfig:
##########
@@ -13,9 +20,63 @@ config RP2040_UART0
 
 if RP2040_UART0
 
+config RP2040_UART0_TX_GPIO

Review Comment:
   I guess I don't understand why one would want to select SPI0 one place and have to go somewhere else to configure it.  That's one thing that I find particularly annoying in the NuttX configuration tool.  
   
   I could understand having these configurations at the board level if the configuration tool had a convenient way to limit numeric input to only a specific number of discreet choices, so that unimplemented choices could not be made. But even then, I would think both selection and configuration should be in the same place.
   
   Is there some advantage to the end user for having the configuration options split up?
   
   Anyway, like I said, I'm open to moving the configuration to the board menu.



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

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] curuvar commented on a diff in pull request #6648: Added Adafruit QT Py RP2040 board and made UART, SPI, and I2C configuration more flexible.

Posted by GitBox <gi...@apache.org>.
curuvar commented on code in PR #6648:
URL: https://github.com/apache/incubator-nuttx/pull/6648#discussion_r927017908


##########
arch/arm/src/rp2040/Kconfig:
##########
@@ -13,9 +20,63 @@ config RP2040_UART0
 
 if RP2040_UART0
 
+config RP2040_UART0_TX_GPIO

Review Comment:
   I moved the pin definitions to boards/arm/rp2040/common/Kconfig as requested.



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

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] davids5 commented on a diff in pull request #6648: Added Adafruit QT Py RP2040 board and made UART, SPI, and I2C configuration more flexible.

Posted by GitBox <gi...@apache.org>.
davids5 commented on code in PR #6648:
URL: https://github.com/apache/incubator-nuttx/pull/6648#discussion_r926862280


##########
arch/arm/src/rp2040/Kconfig:
##########
@@ -13,9 +20,63 @@ config RP2040_UART0
 
 if RP2040_UART0
 
+config RP2040_UART0_TX_GPIO

Review Comment:
   It is a layered approach.
   
   From a schematic we get "what" is wired
   
   from a SW use case we get "How" it is used
    



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

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 merged pull request #6648: Added Adafruit QT Py RP2040 board and made UART, SPI, and I2C configuration more flexible.

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6648:
URL: https://github.com/apache/incubator-nuttx/pull/6648


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

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6648: Added Adafruit QT Py RP2040 board and made UART, SPI, and I2C configuration more flexible.

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6648:
URL: https://github.com/apache/incubator-nuttx/pull/6648#discussion_r926642983


##########
arch/arm/src/rp2040/Kconfig:
##########
@@ -13,9 +20,63 @@ config RP2040_UART0
 
 if RP2040_UART0
 
+config RP2040_UART0_TX_GPIO

Review Comment:
   I am fine with either. @pkarashchenko @acassis what's your opinion?



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

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6648: Added Adafruit QT Py RP2040 board and made UART, SPI, and I2C configuration more flexible.

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6648:
URL: https://github.com/apache/incubator-nuttx/pull/6648#discussion_r926217634


##########
arch/arm/src/rp2040/Kconfig:
##########
@@ -13,9 +20,63 @@ config RP2040_UART0
 
 if RP2040_UART0
 
+config RP2040_UART0_TX_GPIO

Review Comment:
   RP2040_UART0_TX_GPIO is only used in the board file, how about we create a top level Kconfig at:
   boards/arm/rp2040/Kconfig or
   boards/arm/rp2040/common/Kconfig
   to define this common option.



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

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org