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 2021/12/12 03:28:30 UTC

[GitHub] [incubator-nuttx] nandojve opened a new pull request #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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


   ## Summary
   
   Add common folder to share common code between samv7 boards. This will avoid both duplicate and copy and paste practices. This is a sequence which:
   
   - Define internal memories sizes by SoC version. This allows unify linker scripts using a template model.
   - Move all scripts and tools to common folder. It adapts current boards to use the common infrastructure and linker script template.
   - Move all MCUboot related code to common folder and drop all duplicated code and defines.
   - Add MCUboot support to samv71-xult board.
   
   ## Impact
   
   samv7 boards.
   
   ## Testing
   
   - PassCI
   - samv7-xult


-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/same70-qmtech/configs/mcuboot-loader/defconfig
##########
@@ -44,13 +46,11 @@ CONFIG_RAM_SIZE=262144
 CONFIG_RAM_START=0x20400000
 CONFIG_RAW_BINARY=y
 CONFIG_RR_INTERVAL=200
-CONFIG_SAME70QMTECH_FORMAT_MCUBOOT=y
-CONFIG_SAME70QMTECH_MCUBOOT_BOOTLOADER=y
+CONFIG_SAMV7_FORMAT_MCUBOOT=y

Review comment:
       ```suggestion
   CONFIG_SAMV7_FORMAT_MCUBOOT=y
   CONFIG_SAMV7_MCUBOOT_BOOTLOADER=y
   ```
   

##########
File path: boards/arm/samv7/common/src/sam_progmem_common.c
##########
@@ -196,7 +184,7 @@ static int init_ota_partitions(void)
 
   return ret;
 }
-#endif
+#endif /* CONFIG_SAMV7_PROGMEM_MCUBOOT_PARTITION */

Review comment:
       We need to stick to one of the names either `OTA_PARTITION` or `MCUBOOT_PARTITION`.

##########
File path: boards/arm/samv7/same70-xplained/README.txt
##########
@@ -1716,8 +1716,7 @@ Configuration sub-directories
       CONFIG_MCUBOOT_BOOTLOADER=y
       CONFIG_MCUBOOT_ENABLE_LOGGING=y
 
-      CONFIG_SAME70XPLAINED_FORMAT_MCUBOOT=y
-      CONFIG_SAME70XPLAINED_MCUBOOT_BOOTLOADER=y
+      CONFIG_SAMV7_FORMAT_MCUBOOT=y

Review comment:
       ```suggestion
         CONFIG_SAMV7_FORMAT_MCUBOOT=y
         CONFIG_SAMV7_MCUBOOT_BOOTLOADER=y
   ```
   




-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       Yeah, but why do we need `MTD_CONFIG_ERASEDVALUE` to be enabled for MCUboot?




-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/Kconfig
##########
@@ -3326,6 +3326,10 @@ endif
 
 comment "Board-Common Options"
 
+if ARCH_CHIP_SAMV7
+source "boards/arm/samv7/common/Kconfig"
+endif
+

Review comment:
       ```suggestion
   ```




-- 
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] nandojve commented on a change in pull request #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       Dropped MTD_CONFIG




-- 
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] nandojve commented on a change in pull request #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       MTD_CONFIG enables MTD_CONFIG_ERASEDVALUE https://github.com/apache/incubator-nuttx/blob/51a2db6ffc4660346dddf6b577089d6f1dc9c504/drivers/mtd/Kconfig#L181




-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       Yes, please explain. What I read about `MCUBOOT_DEFAULT_FLASH_ERASE_STATE` in `apps` repo is:
   ```
   config MTD_CONFIG_ERASEDVALUE
   	hex "Erased value of bytes on the MTD device"
   	default 0xff
   	---help---
   		Specifies the value of the erased state of the MTD FLASH.  For
   		most FLASH parts, this is 0xff, but could also be zero depending
   		on the device.
   ```
   And per my understanding has nothing to do with `MTD_CONFIG_ERASEDVALUE`.
   `MTD_CONFIG` is:
   ```
   config MTD_CONFIG
   	bool "Enable Dev Config (MTD based) device"
   	default n
   	---help---
   		Provides a /dev/config device for saving / restoring application
   		configuration data to a standard MTD device or partition.
   ```
   and it is used in `sam_at24config.c` for `at24` based example.
   The MCUboot relies on `CONFIG_MTD_PROGMEM_ERASESTATE` that supplies flash erase value `ioctl(MTDIOC_ERASESTATE)`.




-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       I will recheck on qmtech board and reply with my approve




-- 
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] nandojve commented on a change in pull request #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/src/sam_progmem_common.c
##########
@@ -196,7 +184,7 @@ static int init_ota_partitions(void)
 
   return ret;
 }
-#endif
+#endif /* CONFIG_SAMV7_PROGMEM_MCUBOOT_PARTITION */

Review comment:
       There is no intention to change names right now. I missed in rebase, tks!




-- 
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] acassis merged pull request #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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


   


-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       Repeating again that I'm not talking about dependency of `CONFIG_MTD_PROGMEM_ERASESTATE` or `MCUBOOT_DEFAULT_FLASH_ERASE_STATE`. My initial comment is that `SAMV7_PROGMEM_OTA_PARTITION` should not select `MTD_CONFIG` because there is not dependency between this two options because MCUboot partition is located in MTD PROGMEM dev.
   In case if MCUboot partition is located in MTD CONFIG dev, then we can have such dependency.




-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/samv71-xult/configs/mcuboot-nsh/defconfig
##########
@@ -5,81 +5,59 @@
 # You can then do "make savedefconfig" to generate a new defconfig file that includes your
 # modifications.
 #
-# CONFIG_MMCSD_MMCSUPPORT is not set
-# CONFIG_MMCSD_SPI is not set
 # CONFIG_SAMV7_UART0 is not set
 # CONFIG_SAMV7_UART2 is not set
 # CONFIG_SAMV7_UART4 is not set
 CONFIG_ARCH="arm"
-CONFIG_ARCH_BOARD_CUSTOM=y
-CONFIG_ARCH_BOARD_CUSTOM_DIR="../autococo2-board"
-CONFIG_ARCH_BOARD_CUSTOM_DIR_RELPATH=y
-CONFIG_ARCH_BOARD_CUSTOM_NAME="ARCH_BOARD_AUTOCOCO2_BOARD"
+CONFIG_ARCH_BOARD="samv71-xult"
+CONFIG_ARCH_BOARD_SAMV71_XULT=y
+CONFIG_ARCH_BUTTONS=y
 CONFIG_ARCH_CHIP="samv7"
-CONFIG_ARCH_CHIP_SAME70=y
-CONFIG_ARCH_CHIP_SAME70Q21=y
-CONFIG_ARCH_CHIP_SAME70Q=y
+CONFIG_ARCH_CHIP_SAMV71=y
+CONFIG_ARCH_CHIP_SAMV71Q21=y
+CONFIG_ARCH_CHIP_SAMV71Q=y
 CONFIG_ARCH_CHIP_SAMV7=y
+CONFIG_ARCH_CHIP_SAMV7_MEM_FLASH=0x200000
+CONFIG_ARCH_CHIP_SAMV7_MEM_RAM=0x60000
 CONFIG_ARCH_INTERRUPTSTACK=2048
+CONFIG_ARCH_IRQBUTTONS=y
 CONFIG_ARCH_STACKDUMP=y
 CONFIG_ARMV7M_DCACHE=y
 CONFIG_ARMV7M_ICACHE=y
 CONFIG_ARMV7M_LAZYFPU=y
-CONFIG_AT24XX_ADDR=0x57
-CONFIG_AT24XX_EXTENDED=y
-CONFIG_AT24XX_EXTSIZE=160
-CONFIG_AT24XX_SIZE=2
+CONFIG_BOARDCTL_RESET=y
 CONFIG_BOARD_LATE_INITIALIZE=y
 CONFIG_BOARD_LOOPSPERMSEC=51262
 CONFIG_BOOT_MCUBOOT=y
 CONFIG_BUILTIN=y
-CONFIG_DEBUG_FULLOPT=y
-CONFIG_DEBUG_SYMBOLS=y
-CONFIG_FAT_LCNAMES=y
-CONFIG_FAT_LFN=y
-CONFIG_FS_FAT=y
 CONFIG_FS_PROCFS=y
-CONFIG_I2CTOOL_MAXBUS=0
-CONFIG_INTELHEX_BINARY=y
 CONFIG_MCUBOOT_SLOT_CONFIRM_EXAMPLE=y
-CONFIG_MCUBOOT_VERSION="1a9c6d8495e4dbe7d02edf14bb8a9fa1d4e955c0"
-CONFIG_MMCSD_MULTIBLOCK_DISABLE=y
-CONFIG_MMCSD_SDIO=y
-CONFIG_MTD=y
-CONFIG_MTD_AT24XX=y
-CONFIG_MTD_AT25=y

Review comment:
       No. That is fine. Just was thinking `mcuboot-nsh` as an extension of `nsh` with MCUboot capabilities, so was curious why this config is different from `nsh` one




-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       Yeah, but why do we need `MTD_CONFIG_ERASEDVALUE`?




-- 
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] nandojve commented on a change in pull request #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       I understand that MCUBOOT_DEFAULT_FLASH_ERASE_STATE should be dropped in favor to use the well defined Kconfig at NuttX, not at Apps, to initialize structures, as below:
   
   https://github.com/mcu-tools/mcuboot/blob/e9c6b4d7af7598fbd915868cf89383ff854d2241/boot/nuttx/src/flash_map_backend/flash_map_backend.c#L85-L99
   
   I almost sure now that I interpreted wrongly the use of MTD_CONFIG_ERASEDVALUE. The thing is that it is important define a way that serves for all MTD use cases. The CONFIG_MTD_PROGMEM_ERASESTATE won't work to replace MCUBOOT_DEFAULT_FLASH_ERASE_STATE. In my view this should be supplied by NuttX to any bootloader App instead rely on user to configure this.
   
   Anyway, this won't be part of this PR as already stated earlier.




-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help

Review comment:
       ```suggestion
   	---help---
   ```




-- 
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] nandojve commented on a change in pull request #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/samv71-xult/configs/mcuboot-nsh/defconfig
##########
@@ -5,81 +5,59 @@
 # You can then do "make savedefconfig" to generate a new defconfig file that includes your
 # modifications.
 #
-# CONFIG_MMCSD_MMCSUPPORT is not set
-# CONFIG_MMCSD_SPI is not set
 # CONFIG_SAMV7_UART0 is not set
 # CONFIG_SAMV7_UART2 is not set
 # CONFIG_SAMV7_UART4 is not set
 CONFIG_ARCH="arm"
-CONFIG_ARCH_BOARD_CUSTOM=y
-CONFIG_ARCH_BOARD_CUSTOM_DIR="../autococo2-board"
-CONFIG_ARCH_BOARD_CUSTOM_DIR_RELPATH=y
-CONFIG_ARCH_BOARD_CUSTOM_NAME="ARCH_BOARD_AUTOCOCO2_BOARD"
+CONFIG_ARCH_BOARD="samv71-xult"
+CONFIG_ARCH_BOARD_SAMV71_XULT=y
+CONFIG_ARCH_BUTTONS=y
 CONFIG_ARCH_CHIP="samv7"
-CONFIG_ARCH_CHIP_SAME70=y
-CONFIG_ARCH_CHIP_SAME70Q21=y
-CONFIG_ARCH_CHIP_SAME70Q=y
+CONFIG_ARCH_CHIP_SAMV71=y
+CONFIG_ARCH_CHIP_SAMV71Q21=y
+CONFIG_ARCH_CHIP_SAMV71Q=y
 CONFIG_ARCH_CHIP_SAMV7=y
+CONFIG_ARCH_CHIP_SAMV7_MEM_FLASH=0x200000
+CONFIG_ARCH_CHIP_SAMV7_MEM_RAM=0x60000
 CONFIG_ARCH_INTERRUPTSTACK=2048
+CONFIG_ARCH_IRQBUTTONS=y
 CONFIG_ARCH_STACKDUMP=y
 CONFIG_ARMV7M_DCACHE=y
 CONFIG_ARMV7M_ICACHE=y
 CONFIG_ARMV7M_LAZYFPU=y
-CONFIG_AT24XX_ADDR=0x57
-CONFIG_AT24XX_EXTENDED=y
-CONFIG_AT24XX_EXTSIZE=160
-CONFIG_AT24XX_SIZE=2
+CONFIG_BOARDCTL_RESET=y
 CONFIG_BOARD_LATE_INITIALIZE=y
 CONFIG_BOARD_LOOPSPERMSEC=51262
 CONFIG_BOOT_MCUBOOT=y
 CONFIG_BUILTIN=y
-CONFIG_DEBUG_FULLOPT=y
-CONFIG_DEBUG_SYMBOLS=y
-CONFIG_FAT_LCNAMES=y
-CONFIG_FAT_LFN=y
-CONFIG_FS_FAT=y
 CONFIG_FS_PROCFS=y
-CONFIG_I2CTOOL_MAXBUS=0
-CONFIG_INTELHEX_BINARY=y
 CONFIG_MCUBOOT_SLOT_CONFIRM_EXAMPLE=y
-CONFIG_MCUBOOT_VERSION="1a9c6d8495e4dbe7d02edf14bb8a9fa1d4e955c0"
-CONFIG_MMCSD_MULTIBLOCK_DISABLE=y
-CONFIG_MMCSD_SDIO=y
-CONFIG_MTD=y
-CONFIG_MTD_AT24XX=y
-CONFIG_MTD_AT25=y

Review comment:
       It not seems to be relevant in my view. Is there any particular purpose to add those?




-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/Kconfig
##########
@@ -3326,6 +3326,10 @@ endif
 
 comment "Board-Common Options"
 
+if ARCH_CHIP_SAMV7
+source "boards/arm/samv7/common/Kconfig"
+endif
+

Review comment:
       ```suggestion
   ```

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help

Review comment:
       ```suggestion
   	---help---
   ```

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       please double check if `MTD_CONFIG` is mandatory. I think it can be removed from the list

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help

Review comment:
       ```suggestion
   	---help---
   ```

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help
+		 The MCUboot support of loading the firmware images.
+
+if SAMV7_FORMAT_MCUBOOT
+
+config SAMV7_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	select BOARDCTL
+	select BOARDCTL_BOOT_IMAGE
+	help
+		Build the MCUboot first stage bootloader application. This
+		application is the first application that will run after reset.
+		To build normal applications that MCUboot should test and run,
+		left this option untouched.
+
+comment "MCUboot Application Image OTA Update support"
+
+config SAMV7_OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"

Review comment:
       ```suggestion
   	default 0x20000
   ```

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help
+		 The MCUboot support of loading the firmware images.
+
+if SAMV7_FORMAT_MCUBOOT
+
+config SAMV7_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	select BOARDCTL
+	select BOARDCTL_BOOT_IMAGE
+	help
+		Build the MCUboot first stage bootloader application. This
+		application is the first application that will run after reset.
+		To build normal applications that MCUboot should test and run,
+		left this option untouched.
+
+comment "MCUboot Application Image OTA Update support"
+
+config SAMV7_OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config SAMV7_OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config SAMV7_OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x48000"  if SAMV7_MEM_FLASH_512
+	default "0x80000"  if SAMV7_MEM_FLASH_1024
+	default "0x100000" if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config SAMV7_OTA_SLOT_SIZE
+	hex "MCUboot application image slot size (in bytes)"
+	default "0x28000"  if SAMV7_MEM_FLASH_512
+	default "0x60000"  if SAMV7_MEM_FLASH_1024
+	default "0xe0000"  if SAMV7_MEM_FLASH_2048

Review comment:
       ```suggestion
   	default 0x28000  if SAMV7_MEM_FLASH_512
   	default 0x60000  if SAMV7_MEM_FLASH_1024
   	default 0xe0000  if SAMV7_MEM_FLASH_2048
   ```

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help
+		 The MCUboot support of loading the firmware images.
+
+if SAMV7_FORMAT_MCUBOOT
+
+config SAMV7_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	select BOARDCTL
+	select BOARDCTL_BOOT_IMAGE
+	help

Review comment:
       ```suggestion
   	---help---
   ```

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help
+		 The MCUboot support of loading the firmware images.
+
+if SAMV7_FORMAT_MCUBOOT
+
+config SAMV7_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	select BOARDCTL
+	select BOARDCTL_BOOT_IMAGE
+	help
+		Build the MCUboot first stage bootloader application. This
+		application is the first application that will run after reset.
+		To build normal applications that MCUboot should test and run,
+		left this option untouched.
+
+comment "MCUboot Application Image OTA Update support"
+
+config SAMV7_OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config SAMV7_OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config SAMV7_OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x48000"  if SAMV7_MEM_FLASH_512
+	default "0x80000"  if SAMV7_MEM_FLASH_1024
+	default "0x100000" if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config SAMV7_OTA_SLOT_SIZE
+	hex "MCUboot application image slot size (in bytes)"
+	default "0x28000"  if SAMV7_MEM_FLASH_512
+	default "0x60000"  if SAMV7_MEM_FLASH_1024
+	default "0xe0000"  if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SCRATCH_OFFSET
+	hex "MCUboot scratch partition offset"
+	default "0x70000"  if SAMV7_MEM_FLASH_512
+	default "0xe0000"  if SAMV7_MEM_FLASH_1024
+	default "0x1e0000" if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config SAMV7_OTA_SCRATCH_SIZE
+	hex "MCUboot scratch partition size (in bytes)"
+	default "0x10000"  if SAMV7_MEM_FLASH_512
+	default "0x20000"

Review comment:
       Is dependency on `SAMV7_MEM_FLASH_1024` and `SAMV7_MEM_FLASH_2048` missing or `0x20000` is intentional value for both?

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help
+		 The MCUboot support of loading the firmware images.
+
+if SAMV7_FORMAT_MCUBOOT
+
+config SAMV7_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	select BOARDCTL
+	select BOARDCTL_BOOT_IMAGE
+	help
+		Build the MCUboot first stage bootloader application. This
+		application is the first application that will run after reset.
+		To build normal applications that MCUboot should test and run,
+		left this option untouched.
+
+comment "MCUboot Application Image OTA Update support"
+
+config SAMV7_OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config SAMV7_OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config SAMV7_OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x48000"  if SAMV7_MEM_FLASH_512
+	default "0x80000"  if SAMV7_MEM_FLASH_1024
+	default "0x100000" if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config SAMV7_OTA_SLOT_SIZE
+	hex "MCUboot application image slot size (in bytes)"
+	default "0x28000"  if SAMV7_MEM_FLASH_512
+	default "0x60000"  if SAMV7_MEM_FLASH_1024
+	default "0xe0000"  if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SCRATCH_OFFSET
+	hex "MCUboot scratch partition offset"
+	default "0x70000"  if SAMV7_MEM_FLASH_512
+	default "0xe0000"  if SAMV7_MEM_FLASH_1024
+	default "0x1e0000" if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config SAMV7_OTA_SCRATCH_SIZE
+	hex "MCUboot scratch partition size (in bytes)"
+	default "0x10000"  if SAMV7_MEM_FLASH_512

Review comment:
       ```suggestion
   	default 0x10000  if SAMV7_MEM_FLASH_512
   ```

##########
File path: boards/arm/samv7/samv71-xult/configs/mcuboot-nsh/defconfig
##########
@@ -5,81 +5,59 @@
 # You can then do "make savedefconfig" to generate a new defconfig file that includes your
 # modifications.
 #
-# CONFIG_MMCSD_MMCSUPPORT is not set
-# CONFIG_MMCSD_SPI is not set
 # CONFIG_SAMV7_UART0 is not set
 # CONFIG_SAMV7_UART2 is not set
 # CONFIG_SAMV7_UART4 is not set
 CONFIG_ARCH="arm"
-CONFIG_ARCH_BOARD_CUSTOM=y
-CONFIG_ARCH_BOARD_CUSTOM_DIR="../autococo2-board"
-CONFIG_ARCH_BOARD_CUSTOM_DIR_RELPATH=y
-CONFIG_ARCH_BOARD_CUSTOM_NAME="ARCH_BOARD_AUTOCOCO2_BOARD"
+CONFIG_ARCH_BOARD="samv71-xult"
+CONFIG_ARCH_BOARD_SAMV71_XULT=y
+CONFIG_ARCH_BUTTONS=y
 CONFIG_ARCH_CHIP="samv7"
-CONFIG_ARCH_CHIP_SAME70=y
-CONFIG_ARCH_CHIP_SAME70Q21=y
-CONFIG_ARCH_CHIP_SAME70Q=y
+CONFIG_ARCH_CHIP_SAMV71=y
+CONFIG_ARCH_CHIP_SAMV71Q21=y
+CONFIG_ARCH_CHIP_SAMV71Q=y
 CONFIG_ARCH_CHIP_SAMV7=y
+CONFIG_ARCH_CHIP_SAMV7_MEM_FLASH=0x200000
+CONFIG_ARCH_CHIP_SAMV7_MEM_RAM=0x60000
 CONFIG_ARCH_INTERRUPTSTACK=2048
+CONFIG_ARCH_IRQBUTTONS=y
 CONFIG_ARCH_STACKDUMP=y
 CONFIG_ARMV7M_DCACHE=y
 CONFIG_ARMV7M_ICACHE=y
 CONFIG_ARMV7M_LAZYFPU=y
-CONFIG_AT24XX_ADDR=0x57
-CONFIG_AT24XX_EXTENDED=y
-CONFIG_AT24XX_EXTSIZE=160
-CONFIG_AT24XX_SIZE=2
+CONFIG_BOARDCTL_RESET=y
 CONFIG_BOARD_LATE_INITIALIZE=y
 CONFIG_BOARD_LOOPSPERMSEC=51262
 CONFIG_BOOT_MCUBOOT=y
 CONFIG_BUILTIN=y
-CONFIG_DEBUG_FULLOPT=y
-CONFIG_DEBUG_SYMBOLS=y
-CONFIG_FAT_LCNAMES=y
-CONFIG_FAT_LFN=y
-CONFIG_FS_FAT=y
 CONFIG_FS_PROCFS=y
-CONFIG_I2CTOOL_MAXBUS=0
-CONFIG_INTELHEX_BINARY=y
 CONFIG_MCUBOOT_SLOT_CONFIRM_EXAMPLE=y
-CONFIG_MCUBOOT_VERSION="1a9c6d8495e4dbe7d02edf14bb8a9fa1d4e955c0"
-CONFIG_MMCSD_MULTIBLOCK_DISABLE=y
-CONFIG_MMCSD_SDIO=y
-CONFIG_MTD=y
-CONFIG_MTD_AT24XX=y
-CONFIG_MTD_AT25=y

Review comment:
       Why this configuration is not included?

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help
+		 The MCUboot support of loading the firmware images.
+
+if SAMV7_FORMAT_MCUBOOT
+
+config SAMV7_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	select BOARDCTL
+	select BOARDCTL_BOOT_IMAGE
+	help
+		Build the MCUboot first stage bootloader application. This
+		application is the first application that will run after reset.
+		To build normal applications that MCUboot should test and run,
+		left this option untouched.
+
+comment "MCUboot Application Image OTA Update support"
+
+config SAMV7_OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config SAMV7_OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config SAMV7_OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x48000"  if SAMV7_MEM_FLASH_512
+	default "0x80000"  if SAMV7_MEM_FLASH_1024
+	default "0x100000" if SAMV7_MEM_FLASH_2048

Review comment:
       ```suggestion
   	default 0x48000" if SAMV7_MEM_FLASH_512
   	default 0x80000  if SAMV7_MEM_FLASH_1024
   	default 0x100000 if SAMV7_MEM_FLASH_2048
   ```

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help
+		 The MCUboot support of loading the firmware images.
+
+if SAMV7_FORMAT_MCUBOOT
+
+config SAMV7_MCUBOOT_BOOTLOADER

Review comment:
       Actually I reread your previous comments and agree that `SAMV7_MCUBOOT_BOOTLOADER` option is not needed at all and everything can be controlled using `BOARDCTL_BOOT_IMAGE` that is selected by the `MCUBOOT_BOOTLOADER` option from the `app` layer.
   Sorry for confusion. `SAMV7_MCUBOOT_BOOTLOADER` can be safely removed from this PR

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help
+		 The MCUboot support of loading the firmware images.
+
+if SAMV7_FORMAT_MCUBOOT
+
+config SAMV7_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	select BOARDCTL
+	select BOARDCTL_BOOT_IMAGE
+	help
+		Build the MCUboot first stage bootloader application. This
+		application is the first application that will run after reset.
+		To build normal applications that MCUboot should test and run,
+		left this option untouched.
+
+comment "MCUboot Application Image OTA Update support"
+
+config SAMV7_OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config SAMV7_OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config SAMV7_OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x48000"  if SAMV7_MEM_FLASH_512
+	default "0x80000"  if SAMV7_MEM_FLASH_1024
+	default "0x100000" if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config SAMV7_OTA_SLOT_SIZE
+	hex "MCUboot application image slot size (in bytes)"
+	default "0x28000"  if SAMV7_MEM_FLASH_512
+	default "0x60000"  if SAMV7_MEM_FLASH_1024
+	default "0xe0000"  if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SCRATCH_OFFSET
+	hex "MCUboot scratch partition offset"
+	default "0x70000"  if SAMV7_MEM_FLASH_512
+	default "0xe0000"  if SAMV7_MEM_FLASH_1024
+	default "0x1e0000" if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config SAMV7_OTA_SCRATCH_SIZE
+	hex "MCUboot scratch partition size (in bytes)"
+	default "0x10000"  if SAMV7_MEM_FLASH_512
+	default "0x20000"

Review comment:
       ```suggestion
   	default 0x20000
   ```

##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+	select SAMV7_PROGMEM
+	select SAMV7_PROGMEM_ERASESTATE
+
+config SAMV7_MCUBOOT_HEADER_SIZE
+	hex
+	default 0x200
+	depends on SAMV7_FORMAT_MCUBOOT
+
+menuconfig SAMV7_FORMAT_MCUBOOT
+	bool "MCUboot bootable format"
+	default n
+	select SAMV7_PROGMEM_OTA_PARTITION
+	help
+		 The MCUboot support of loading the firmware images.
+
+if SAMV7_FORMAT_MCUBOOT
+
+config SAMV7_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	select BOARDCTL
+	select BOARDCTL_BOOT_IMAGE
+	help
+		Build the MCUboot first stage bootloader application. This
+		application is the first application that will run after reset.
+		To build normal applications that MCUboot should test and run,
+		left this option untouched.
+
+comment "MCUboot Application Image OTA Update support"
+
+config SAMV7_OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config SAMV7_OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config SAMV7_OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x48000"  if SAMV7_MEM_FLASH_512
+	default "0x80000"  if SAMV7_MEM_FLASH_1024
+	default "0x100000" if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config SAMV7_OTA_SLOT_SIZE
+	hex "MCUboot application image slot size (in bytes)"
+	default "0x28000"  if SAMV7_MEM_FLASH_512
+	default "0x60000"  if SAMV7_MEM_FLASH_1024
+	default "0xe0000"  if SAMV7_MEM_FLASH_2048
+
+config SAMV7_OTA_SCRATCH_OFFSET
+	hex "MCUboot scratch partition offset"
+	default "0x70000"  if SAMV7_MEM_FLASH_512
+	default "0xe0000"  if SAMV7_MEM_FLASH_1024
+	default "0x1e0000" if SAMV7_MEM_FLASH_2048

Review comment:
       ```suggestion
   	default 0x70000  if SAMV7_MEM_FLASH_512
   	default 0xe0000  if SAMV7_MEM_FLASH_1024
   	default 0x1e0000 if SAMV7_MEM_FLASH_2048
   ```




-- 
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 #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/samv71-xult/configs/mcuboot-nsh/defconfig
##########
@@ -5,81 +5,59 @@
 # You can then do "make savedefconfig" to generate a new defconfig file that includes your
 # modifications.
 #
-# CONFIG_MMCSD_MMCSUPPORT is not set
-# CONFIG_MMCSD_SPI is not set
 # CONFIG_SAMV7_UART0 is not set
 # CONFIG_SAMV7_UART2 is not set
 # CONFIG_SAMV7_UART4 is not set
 CONFIG_ARCH="arm"
-CONFIG_ARCH_BOARD_CUSTOM=y
-CONFIG_ARCH_BOARD_CUSTOM_DIR="../autococo2-board"
-CONFIG_ARCH_BOARD_CUSTOM_DIR_RELPATH=y
-CONFIG_ARCH_BOARD_CUSTOM_NAME="ARCH_BOARD_AUTOCOCO2_BOARD"
+CONFIG_ARCH_BOARD="samv71-xult"
+CONFIG_ARCH_BOARD_SAMV71_XULT=y
+CONFIG_ARCH_BUTTONS=y
 CONFIG_ARCH_CHIP="samv7"
-CONFIG_ARCH_CHIP_SAME70=y
-CONFIG_ARCH_CHIP_SAME70Q21=y
-CONFIG_ARCH_CHIP_SAME70Q=y
+CONFIG_ARCH_CHIP_SAMV71=y
+CONFIG_ARCH_CHIP_SAMV71Q21=y
+CONFIG_ARCH_CHIP_SAMV71Q=y
 CONFIG_ARCH_CHIP_SAMV7=y
+CONFIG_ARCH_CHIP_SAMV7_MEM_FLASH=0x200000
+CONFIG_ARCH_CHIP_SAMV7_MEM_RAM=0x60000
 CONFIG_ARCH_INTERRUPTSTACK=2048
+CONFIG_ARCH_IRQBUTTONS=y
 CONFIG_ARCH_STACKDUMP=y
 CONFIG_ARMV7M_DCACHE=y
 CONFIG_ARMV7M_ICACHE=y
 CONFIG_ARMV7M_LAZYFPU=y
-CONFIG_AT24XX_ADDR=0x57
-CONFIG_AT24XX_EXTENDED=y
-CONFIG_AT24XX_EXTSIZE=160
-CONFIG_AT24XX_SIZE=2
+CONFIG_BOARDCTL_RESET=y
 CONFIG_BOARD_LATE_INITIALIZE=y
 CONFIG_BOARD_LOOPSPERMSEC=51262
 CONFIG_BOOT_MCUBOOT=y
 CONFIG_BUILTIN=y
-CONFIG_DEBUG_FULLOPT=y
-CONFIG_DEBUG_SYMBOLS=y
-CONFIG_FAT_LCNAMES=y
-CONFIG_FAT_LFN=y
-CONFIG_FS_FAT=y
 CONFIG_FS_PROCFS=y
-CONFIG_I2CTOOL_MAXBUS=0
-CONFIG_INTELHEX_BINARY=y
 CONFIG_MCUBOOT_SLOT_CONFIRM_EXAMPLE=y
-CONFIG_MCUBOOT_VERSION="1a9c6d8495e4dbe7d02edf14bb8a9fa1d4e955c0"
-CONFIG_MMCSD_MULTIBLOCK_DISABLE=y
-CONFIG_MMCSD_SDIO=y
-CONFIG_MTD=y
-CONFIG_MTD_AT24XX=y
-CONFIG_MTD_AT25=y

Review comment:
       No. That is fine. Just was thinking `mcuboot-nsh` as an extension of `nsh` with MCUboot capabilities, so was curious why this config is different from `nsh` one. I'm fine with the current change




-- 
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] nandojve commented on a change in pull request #4981: boards/arm/samv7: Introduce common folder and refactor MCUboot

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



##########
File path: boards/arm/samv7/common/Kconfig
##########
@@ -0,0 +1,96 @@
+#
+# For a description of the syntax of this configuration file,
+# see the file kconfiglanguage.txt in the NuttX tools repository.
+#
+
+config BOARD_SAMV7_COMMON
+	bool "Board common logic"
+	default y
+	help
+		 Board common logic located in each board/common folder.
+
+if BOARD_SAMV7_COMMON
+
+config SAMV7_PROGMEM_OTA_PARTITION
+	bool
+	default n
+	select ARCH_RAMFUNCS
+	select BCH
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_CONFIG

Review comment:
       This store the correct value of CONFIG_MCUBOOT_DEFAULT_FLASH_ERASE_STATE. I'll send an update later dropping it from this sequence. That is a remaining from a previous attempt to reorganize MCUboot. However, it seems more prudent first reorganize samv7 and then propose changes in a future 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