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/03/09 17:20:17 UTC

[GitHub] [incubator-nuttx] pkarashchenko opened a new pull request #5705: tools: add option to use in-tree board-level common code

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


   ## Summary
   This PR fix the failure reported by users after https://github.com/apache/incubator-nuttx/pull/5274
   
   ## Impact
   Custom boards users
   
   ## Testing
   Pass CI


-- 
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 pull request #5705: tools: add option to use in-tree board-level common code

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


   Currently next in-tree board archs have common:
   ```
   ./boards/xtensa/esp32s3/common
   ./boards/xtensa/esp32/common
   ./boards/xtensa/esp32s2/common
   ./boards/arm/rp2040/common
   ./boards/arm/cxd56xx/common
   ./boards/arm/stm32/common
   ./boards/arm/samv7/common
   ./boards/risc-v/mpfs/common
   ```
   and there are only two options available `BOARD_STM32_COMMON` and `BOARD_SAMV7_COMMON`. So we need either to equip the rest of the cases with `BOARD_ARCH_CHIP_COMMON` option go to go with `CONFIG_BOARD_CUSTOM_ARCH_BOARD_COMMON`. Personally I do not know what is better and taking into account next release time frame I'm not sure that I will be able to add  `BOARD_ARCH_CHIP_COMMON` to all missing places. So I made this simple change that covers error reported by users.


-- 
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 pull request #5705: tools: add option to use in-tree board-level common code

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


   > As far as I can tell, these specific options (like `BOARD_STM32_COMMON`) are used only in the common folder's Kconfig file to select the source files to build. Wouldn't be better to just replace it with this new config? We won't have to add other chips options and we won't have to deal with this every-time we introduce the common folder for a family.
   
   The specific options (like `BOARD_STM32_COMMON`) are used by in-tree configs. The in-tree board common is mapped via `$(wildcard $(BOARD_DIR)$(DELIM)..$(DELIM)common)` and not via `$(wildcard $(TOPDIR)$(DELIM)boards$(DELIM)$(CONFIG_ARCH)$(DELIM)$(CONFIG_ARCH_CHIP)$(DELIM)common)`, so with current code we can't eliminate `specific options (like `BOARD_STM32_COMMON`)` because `$(wildcard $(BOARD_DIR)$(DELIM)..$(DELIM)common)` will always exists for STM32 based in-tree boards.
   


-- 
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 pull request #5705: tools: add option to use in-tree board-level common code

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


   I have made alternative PR https://github.com/apache/incubator-nuttx/pull/5712 that seems to be move generic. Please take a look.


-- 
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 pull request #5705: tools: add option to use in-tree board-level common code

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


   > How are the other options going to be used now? I mean those chip specific like STM32_COMMON etc.
   > Can they be replaced by this new option?
   > 
   
   No. Those will coexist. The `STM32_COMMON` is about to use the code or not even for the boards that are located in NuttX repo. So we can have in-tree stm32 based boards that will not use `STM32_COMMON`. I will explore if the option is needed and will issue a separate PR to clean it up.
   
   This patch is intended to address issues reported by users while keeping an option that was added by initial 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.

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] gustavonihei commented on a change in pull request #5705: tools: add option to use in-tree board-level common code

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



##########
File path: boards/Kconfig
##########
@@ -2363,6 +2363,14 @@ config ARCH_BOARD_CUSTOM_DIR_RELPATH
 	---help---
 		Specifies that the board directory is relative to the NuttX directory.
 
+config BOARD_CUSTOM_ARCH_BOARD_COMMON
+	bool "Use in-tree board-level common code"
+	default n
+	---help---
+		Enable this option to use NuttX in-tree board-level common code
+		for selected architecture.  The path to common folder will be set to
+		"boards/$(CONFIG_ARCH)/$(CONFIG_ARCH_CHIP)/common" location

Review comment:
       ```suggestion
   		"boards/$(CONFIG_ARCH)/$(CONFIG_ARCH_CHIP)/common" location.
   ```
   Missing period.




-- 
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 change in pull request #5705: tools: add option to use in-tree board-level common code

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



##########
File path: tools/Config.mk
##########
@@ -149,10 +149,10 @@ ifeq (,$(wildcard $(CUSTOM_BOARD_KPATH)))
 else
   BOARD_KCONFIG = $(CUSTOM_BOARD_KPATH)
 endif
-
-BOARD_COMMON_DIR ?= $(wildcard $(BOARD_DIR)$(DELIM)..$(DELIM)common)
-ifeq ($(BOARD_COMMON_DIR),)
-  BOARD_COMMON_DIR = $(wildcard $(TOPDIR)$(DELIM)boards$(DELIM)$(CONFIG_ARCH)$(DELIM)$(CONFIG_ARCH_CHIP)$(DELIM)common)
+ifeq ($(CONFIG_BOARD_CUSTOM_ARCH_BOARD_COMMON),y)
+  BOARD_COMMON_DIR ?= $(wildcard $(TOPDIR)$(DELIM)boards$(DELIM)$(CONFIG_ARCH)$(DELIM)$(CONFIG_ARCH_CHIP)$(DELIM)common)

Review comment:
       BOARD_COMMON_DIR is used inside Make.defs, if user don't like the built-in common board, he can ignore this variable directly, I think.




-- 
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] hartmannathan commented on pull request #5705: tools: add option to use in-tree board-level common code

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


   I can't actually test this right now, but I like the idea that users can disable the common directory for out-of-tree boards.


-- 
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 closed pull request #5705: tools: add option to use in-tree board-level common code

Posted by GitBox <gi...@apache.org>.
pkarashchenko closed pull request #5705:
URL: https://github.com/apache/incubator-nuttx/pull/5705


   


-- 
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] Ouss4 commented on pull request #5705: tools: add option to use in-tree board-level common code

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


   As far as I can tell, these specific options (like `BOARD_STM32_COMMON`) are used only in the common folder's Kconfig file to select the source files to build.  Wouldn't be better to just replace it with this new config? We won't have to add other chips options and we won't have to deal with this every-time we introduce the common folder for a family.


-- 
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 change in pull request #5705: tools: add option to use in-tree board-level common code

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



##########
File path: boards/Kconfig
##########
@@ -2363,6 +2363,14 @@ config ARCH_BOARD_CUSTOM_DIR_RELPATH
 	---help---
 		Specifies that the board directory is relative to the NuttX directory.
 
+config BOARD_CUSTOM_ARCH_BOARD_COMMON
+	bool "Use in-tree board-level common code"
+	default n
+	---help---
+		Enable this option to use NuttX in-tree board-level common code
+		for selected architecture.  The path to common folder will be set to
+		"boards/$(CONFIG_ARCH)/$(CONFIG_ARCH_CHIP)/common" location

Review comment:
       Done




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