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/11/27 18:04:31 UTC

[GitHub] [incubator-nuttx] tito97sp opened a new pull request #4908: add board files to support mcuboot

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


   Signed-off-by: Andres Sanchez <ti...@hotmail.com>
   
   ## Summary
   This pull request add support of mcuboot secure booting to nucleo h743zi board.
   
   ## Impact
   Two new configurations has been added, 
   
   - mcuboot-loader: app-based bootloader that implements mcuboot functionality
   
   - mcuboot-app: simple app with mcuboot-confirm as main entry application
   
   It needs to be configured in the toolchain the mcuboot image signer applcation "imgtool.py" to automatically sign the images before flashing
   
   ## Testing
   
   


-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       The MCUboot in nuttx-apps has same configuration option: https://github.com/apache/incubator-nuttx-apps/blob/c9f736005ac3278faf67d6c9458101bff7882d46/boot/mcuboot/Kconfig#L38..L44
   I'm not sure if it will work with two configuration options with the same name




-- 
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] tito97sp commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x100000"

Review comment:
       Yeah I know. If I use the entire memory of flash the MTD device fails writing. There must be a BUG in stm32_flash.c file when trying to work with the last sectors of each flash bank.
   
   I will try to solve it ASAP




-- 
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 #4908: nucleo-h743zi: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include "nucleo-h743zi.h"
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_STM32_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_STM32_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_STM32_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_STM32_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                               uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);

Review comment:
       Why
   ```
    
    ​  ​ASSERT​((mtd_offset + mtd_size) <= ​up_progmem_neraseblocks​() * 
    ​          ​up_progmem_pagesize​(​0​));
   ```
   has been removed? Is it failing on this target?

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include "nucleo-h743zi.h"
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_STM32_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_STM32_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_STM32_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_STM32_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                               uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+    {
+      return NULL;
+    }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+    {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+        {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+        }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+        {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+        }
+#endif
+    }
+
+  return ret;
+}
+#endif /* CONFIG_STM32_PROGMEM_OTA_PARTITION */
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+
+  stm32h7_flash_unlock();

Review comment:
       I see that flash lock/unlock is managed by stm32h7 progmem layer. Why do we need this call here?




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,141 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_ARM_MPU_EARLY_RESET=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_NET=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_ETH0_PHY_LAN8742A=y
+CONFIG_EXAMPLES_LEDS=y
+CONFIG_EXAMPLES_LEDS_LEDSET=0x03
+CONFIG_EXAMPLES_TCPECHO=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FS_ROMFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_I2C=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_THROTTLE=0
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_SLOT_CONFIRM_EXAMPLE=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"
+CONFIG_MMCSD=y
+CONFIG_MM_CIRCBUF=y
+CONFIG_MM_REGIONS=4
+CONFIG_MTD=y
+CONFIG_MTD_PARTITION=y
+CONFIG_MTD_PROGMEM=y

Review comment:
       I see that you defaulted `APP_FORMAT_MCUBOOT` to `y`, then should other configs be regenerated to add `# CONFIG_APP_FORMAT_MCUBOOT is not set`?




-- 
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 commented on pull request #4908: add board files to support mcuboot

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


   Hi @tito97sp please fix these small issues:
   ```
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_boot_image.c:2:1: error: Relative file path does not match actual file
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_boot_image.c:106:2: error: C++ style comment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_boot_image.c:110:2: error: C++ style comment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_boot_image.c:111:0: error: TABs found.  First detected
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_boot_image.c:112:1: error: Right brace must be followed by a blank line
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_boot_image.c:168:0: error: Garbage follows right bracket
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_bringup.c:329:4: error: Bad alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_bringup.c:330:6: error: Bad left brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_bringup.c:331:79: error: Long line found
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_bringup.c:333:6: error: Bad right brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:2:1: error: Relative file path does not match actual file
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:119:1: error: Too many blank lines
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:147:2: error: C++ style comment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:148:2: error: C++ style comment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:157:2: error: Bad left brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:159:2: error: Bad right brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:189:2: error: Bad left brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:195:6: error: Bad left brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:198:6: error: Bad right brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:205:6: error: Bad left brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:208:6: error: Bad right brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:210:2: error: Bad right brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:216:1: error: Too many blank lines
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:232:1: error: Too many blank lines
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:233:27: error: C++ style comment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:233:35: error: Mixed case identifier found
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:233:43: error: Mixed case identifier found
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:233:80: error: Long line found
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:239:2: error: Bad left brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:240:4: error: Bad alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:241:2: error: Bad right brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:246:2: error: Bad left brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:247:4: error: Bad alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:248:2: error: Bad right brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:254:2: error: Bad left brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:257:4: error: Bad alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:258:2: error: Bad right brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:273:2: error: Bad left brace alignment
   Error: /home/runner/work/incubator-nuttx/incubator-nuttx/nuttx/boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c:277:2: error: Bad right brace alignment
   ```
   You can run nxstyle/checkpatch to confirm everything is fine


-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_boot_image.c
##########
@@ -0,0 +1,168 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_reset.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <debug.h>
+#include <stdio.h>
+#include <fcntl.h>
+
+#include <sys/boardctl.h>
+#include <nuttx/irq.h>
+#include <nuttx/cache.h>
+
+#include "nvic.h"
+#include "arm_arch.h"
+#include "barriers.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure represents the first two entries on NVIC vector table */
+
+struct arm_vector_table
+{
+  uint32_t spr;   /* Stack pointer on reset */
+  uint32_t reset; /* Pointer to reset exception handler */
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static void cleanup_arm_nvic(void);
+static void systick_disable(void);
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name:  cleanup_arm_nvic
+ *
+ * Description:
+ *   Acknowledge and disable all interrupts in NVIC
+ *
+ * Input Parameters:
+ *   None
+ *
+ *  Returned Value:
+ *    None
+ *
+ ****************************************************************************/
+
+static void cleanup_arm_nvic(void)
+{
+  int i;
+
+  /* Allow any pending interrupts to be recognized */
+
+  ARM_ISB();
+  cpsid();
+
+  /* Disable all interrupts */
+
+  for (i = 0; i < NR_IRQS; i += 32)
+    {
+      putreg32(0xffffffff, NVIC_IRQ_CLEAR(i));
+    }
+
+  /* Clear all pending interrupts */
+
+  for (i = 0; i < NR_IRQS; i += 32)
+    {
+      putreg32(0xffffffff, NVIC_IRQ_CLRPEND(i));
+    }
+}
+
+static void systick_disable(void)

Review comment:
       Please take a look into https://github.com/apache/incubator-nuttx/pull/4912




-- 
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 pull request #4908: nucleo-h743zi: add board files to support mcuboot

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


   Hi @tito97sp ,
   
   I add #4939 with the intention to clean a little bit the MCUboot support for samv7. I think better improvement is stop to duplicate code between boards and have only one linker script based in a template model.


-- 
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 edited a comment on pull request #4908: nucleo-h743zi: add board files to support mcuboot

Posted by GitBox <gi...@apache.org>.
nandojve edited a comment on pull request #4908:
URL: https://github.com/apache/incubator-nuttx/pull/4908#issuecomment-986043881


   Hi @tito97sp ,
   
   I add #4939 with the intention to clean a little bit the MCUboot support for samv7. I think that change may help to avoid duplicate code between boards and have only one linker script based in a template model.


-- 
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 #4908: nucleo-h743zi: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcuboot-app.ld
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcuboot-app.ld
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The STM32H743ZI has 2048Kb of main FLASH memory. The flash memory is
+ * partitioned into a User Flash memory and a System Flash memory. Each
+ * of these memories has two banks:
+ *
+ *   1) User Flash memory:
+ *
+ *      Bank 1: Start address 0x0800:0000 to 0x080F:FFFF with 8 sectors, 128Kb each
+ *      Bank 2: Start address 0x0810:0000 to 0x081F:FFFF with 8 sectors, 128Kb each
+ *
+ *   2) System Flash memory:
+ *
+ *      Bank 1: Start address 0x1FF0:0000 to 0x1FF1:FFFF with 1 x 128Kb sector
+ *      Bank 1: Start address 0x1FF4:0000 to 0x1FF5:FFFF with 1 x 128Kb sector
+ *
+ *   3) User option bytes for user configuration, only in Bank 1.
+ *
+ * In the STM32H743ZI, two different boot spaces can be selected through
+ * the BOOT pin and the boot base address programmed in the BOOT_ADD0 and
+ * BOOT_ADD1 option bytes:
+ *
+ *   1) BOOT=0: Boot address defined by user option byte BOOT_ADD0[15:0].
+ *      ST programmed value: Flash memory at 0x0800:0000
+ *   2) BOOT=1: Boot address defined by user option byte BOOT_ADD1[15:0].
+ *      ST programmed value: System bootloader at 0x1FF0:0000
+ *
+ * TODO: Check next paragraph with nucleo schematics
+ *
+ * NuttX does not modify these option bytes. On the unmodified NUCLEO-H743ZI
+ * board, the BOOT0 pin is at ground so by default, the STM32 will boot
+ * to address 0x0800:0000 in FLASH.
+ *
+ * The STM32H743ZI also has 1024Kb of data SRAM.
+ * SRAM is split up into several blocks and into three power domains:
+ *
+ *   1) TCM SRAMs are dedicated to the Cortex-M7 and are accessible with
+ *      0 wait states by the Cortex-M7 and by MDMA through AHBS slave bus
+ *
+ *      1.1) 128Kb of DTCM-RAM beginning at address 0x2000:0000
+ *
+ *           The DTCM-RAM is organized as 2 x 64Kb DTCM-RAMs on 2 x 32 bit
+ *           DTCM ports. The DTCM-RAM could be used for critical real-time
+ *           data, such as interrupt service routines or stack / heap memory.
+ *           Both DTCM-RAMs can be used in parallel (for load/store operations)
+ *           thanks to the Cortex-M7 dual issue capability.
+ *
+ *      1.2)  64Kb of ITCM-RAM beginning at address 0x0000:0000
+ *
+ *           This RAM is connected to ITCM 64-bit interface designed for
+ *           execution of critical real-times routines by the CPU.
+ *
+ *   2) AXI SRAM (D1 domain) accessible by all system masters except BDMA
+ *      through D1 domain AXI bus matrix
+ *
+ *      2.1) 512Kb of SRAM beginning at address 0x2400:0000
+ *
+ *   3) AHB SRAM (D2 domain) accessible by all system masters except BDMA
+ *      through D2 domain AHB bus matrix
+ *
+ *      3.1) 128Kb of SRAM1 beginning at address 0x3000:0000
+ *      3.2) 128Kb of SRAM2 beginning at address 0x3002:0000
+ *      3.3)  32Kb of SRAM3 beginning at address 0x3004:0000
+ *
+ *      SRAM1 - SRAM3 are one contiguous block: 288Kb at address 0x3000:0000
+ *
+ *   4) AHB SRAM (D3 domain) accessible by most of system masters
+ *      through D3 domain AHB bus matrix
+ *
+ *      4.1)  64Kb of SRAM4 beginning at address 0x3800:0000
+ *      4.1)   4Kb of backup RAM beginning at address 0x3880:0000
+ *
+ * When booting from FLASH, FLASH memory is aliased to address 0x0000:0000
+ * where the code expects to begin execution by jumping to the entry point in
+ * the 0x0800:0000 address range.
+ */
+
+MEMORY
+{
+  itcm  (rwx) : ORIGIN = 0x00000000, LENGTH =   64K
+  flash (rx)  : ORIGIN = 0x08020000, LENGTH = 1920K

Review comment:
       Also according to
   ```
   config STM32_OTA_SLOT_SIZE
   	hex "MCUboot application image slot size (in bytes)"
   	default "0xc0000"
   ```
   the `LENGTH` should be `768K` and not `1920K`




-- 
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] tito97sp commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       I have some doubts respect this CONFIG flag. When selected it indicates the linker to use the `xxx-mcuboot-loader.ld` linker script in place of `xxx-mcuboot-app.ld`. This is confusing because of the word **APP** in the "CONFIG_APP_MCUBOOT_BOOTLOADER" flag. 
   
   Has you think in using the inverse logic??
   CONFIG_APP_MCUBOOT_BOOTLOADER=y     --> use xxx-mcuboot-app.ld
   CONFIG_APP_MCUBOOT_BOOTLOADER=y     --> use xxx-mcuboot-laoder.ld
   
   




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       The MCUboot in nuttx-apps has same configuration option:
   https://github.com/apache/incubator-nuttx-apps/blob/c9f736005ac3278faf67d6c9458101bff7882d46/boot/mcuboot/Kconfig#L38..L44
   ```
   config MCUBOOT_BOOTLOADER
   	bool "MCUboot bootloader application"
   	default n
   	select BOARDCTL
   	select BOARDCTL_BOOT_IMAGE
   	---help---
   		MCUboot bootloader application
   ```
   I'm not sure if it will work with two configuration options with the same name




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       The MCUboot in nuttx-apps has same configuration option. Please see https://github.com/apache/incubator-nuttx-apps/blob/c9f736005ac3278faf67d6c9458101bff7882d46/boot/mcuboot/Kconfig#L38..L44
   I'm not sure if it will work with two configuration options with the same name




-- 
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] tito97sp commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       I like the option of renaming both APP_FORMAT_MCUBOOT and APP_MCUBOOT_BOOTLOADER to STM32_FORMAT_MCUBOOT and STM32_MCUBOOT_BOOTLOADER. 
   
   Nevertheless as they are not specific configurations of STM32 boards I think they could be confusing in the future. Maybe it should be a standardized way of configuring the mcuboot for every boards and archs. 

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_APP_FORMAT_MCUBOOT

Review comment:
       Yes sorry. I forget. 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



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       The MCUboot in nuttx-apps has same configuration option:
   https://github.com/apache/incubator-nuttx-apps/blob/c9f736005ac3278faf67d6c9458101bff7882d46/boot/mcuboot/Kconfig#L38
   ```
   config MCUBOOT_BOOTLOADER
   	bool "MCUboot bootloader application"
   	default n
   	select BOARDCTL
   	select BOARDCTL_BOOT_IMAGE
   	---help---
   		MCUboot bootloader application
   ```
   I'm not sure if it will work with two configuration options with the same name




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       https://github.com/apache/incubator-nuttx-apps/blob/master/boot/mcuboot/Kconfig#L38..L44 has same configuration option
   ```
   config MCUBOOT_BOOTLOADER
   	bool "MCUboot bootloader application"
   	default n
   	select BOARDCTL
   	select BOARDCTL_BOOT_IMAGE
   	---help---
   		MCUboot bootloader application
   ```
   I'm not sure if it will work with two configuration options with the same name




-- 
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] tito97sp commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/Make.defs
##########
@@ -22,7 +22,15 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-LDSCRIPT = flash.ld
+ifeq ($(CONFIG_APP_FORMAT_MCUBOOT),y)
+ifeq ($(CONFIG_MCUBOOT_BOOTLOADER),y)
+    LDSCRIPT = flash-mcuboot-loader.ld
+else
+    LDSCRIPT = flash-mcuboot-app.ld

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



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c

Review comment:
       ```suggestion
    * boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
   ```




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_FORMAT_MCUBOOT
+
+config STM32_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x100000"
+
+config OTA_SCRATCH_OFFSET
+	hex "MCUboot scratch partition offset"
+	default "0x1c0000"
+
+config OTA_SLOT_SIZE

Review comment:
       ```suggestion
   config STM32_OTA_SLOT_SIZE
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_FORMAT_MCUBOOT
+
+config STM32_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x100000"
+
+config OTA_SCRATCH_OFFSET

Review comment:
       ```suggestion
   config STM32_OTA_SCRATCH_OFFSET
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_FORMAT_MCUBOOT
+
+config STM32_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x100000"
+
+config OTA_SCRATCH_OFFSET
+	hex "MCUboot scratch partition offset"
+	default "0x1c0000"
+
+config OTA_SLOT_SIZE
+	hex "MCUboot application image slot size (in bytes)"
+	default "0xc0000"
+
+config OTA_SCRATCH_SIZE

Review comment:
       ```suggestion
   config STM32_OTA_SCRATCH_SIZE
   ```




-- 
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] tito97sp commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-loader/defconfig
##########
@@ -0,0 +1,94 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARDCTL_ROMDISK=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LATE_INITIALIZE=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_NCHAINS=24
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_BOOTLOADER=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"

Review comment:
       I was using the most recent commit of mcuboot trying to avoid some nuttx compilation warnings. What commit/version do you recommend me to use? Thanks 

##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcu-app.ld
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/scripts/flash.ld

Review comment:
       You are right! Thanks

##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/Make.defs
##########
@@ -22,7 +22,15 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-LDSCRIPT = flash.ld
+ifeq ($(CONFIG_APP_FORMAT_MCUBOOT),y)
+ifeq ($(CONFIG_MCUBOOT_BOOTLOADER),y)
+    LDSCRIPT = flash-mcuboot-loader.ld

Review comment:
       I use this approach! Thanks!

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,141 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_ARM_MPU_EARLY_RESET=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARD_CUSTOM_LEDS=y

Review comment:
       Done

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c

Review comment:
       Done

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);

Review comment:
       Done

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)

Review comment:
       Done

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  //ASSERT((mtd_offset + mtd_size) <= up_progmem_neraseblocks() *
+  //        up_progmem_pagesize(0));
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+  {
+      return NULL;
+  }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+  {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+      {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+      }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+      {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+      }
+#endif
+  }
+
+  return ret;
+}
+#endif
+
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+
+
+  stm32h7_flash_unlock();  //TODO: Remove. Is only because cant link if removed.
+
+  /* Create an instance of the SAME70 FLASH program memory device driver */

Review comment:
       Done

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  //ASSERT((mtd_offset + mtd_size) <= up_progmem_neraseblocks() *
+  //        up_progmem_pagesize(0));
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+  {
+      return NULL;
+  }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+  {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+      {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+      }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+      {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+      }
+#endif
+  }
+
+  return ret;
+}
+#endif
+
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+

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



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);

Review comment:
       ```suggestion
                                                  uint32_t mtd_size);
   ```




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,141 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_ARM_MPU_EARLY_RESET=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARD_CUSTOM_LEDS=y

Review comment:
       ```suggestion
   CONFIG_BOARD_CUSTOM_LEDS=y
   CONFIG_BOARD_LATE_INITIALIZE=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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Maybe also reasonable to remove `APP_` from `APP_FORMAT_MCUBOOT` and rename to `STM32_FORMAT_MCUBOOT`. Or go with `CONFIG_STM32_BOOT_MCUBOOT`. That will straightforward be licked with nuttx-app mcuboot option `CONFIG_BOOT_MCUBOOT`.
   What do you 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] pkarashchenko commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Maybe also reasonable to remove `APP_` from `APP_FORMAT_MCUBOOT` and rename to `STM32_FORMAT_MCUBOOT`. Or go with `CONFIG_STM32_BOOT_MCUBOOT`. That will straightforward be licked with nuttx-app mcuboot option `CONFIG_BOOT_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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       I would suggest `CONFIG_APP_MCUBOOT_BOOTLOADER`. Then I can do similar rename for SAMv7 to have style consistent




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       https://github.com/pkarashchenko/incubator-nuttx-apps/blob/master/boot/mcuboot/Kconfig has same configuration option
   ```
   config MCUBOOT_BOOTLOADER
   	bool "MCUboot bootloader application"
   	default n
   	select BOARDCTL
   	select BOARDCTL_BOOT_IMAGE
   	---help---
   		MCUboot bootloader application
   ```
   I'm not sure if it will work with two configuration options with the same name

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-loader/defconfig
##########
@@ -0,0 +1,94 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARDCTL_ROMDISK=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LATE_INITIALIZE=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_NCHAINS=24
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_BOOTLOADER=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"

Review comment:
       ```suggestion
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  //ASSERT((mtd_offset + mtd_size) <= up_progmem_neraseblocks() *
+  //        up_progmem_pagesize(0));

Review comment:
       Why this is commented out?

##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/Make.defs
##########
@@ -22,7 +22,15 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-LDSCRIPT = flash.ld
+ifeq ($(CONFIG_APP_FORMAT_MCUBOOT),y)
+ifeq ($(CONFIG_MCUBOOT_BOOTLOADER),y)
+    LDSCRIPT = flash-mcuboot-loader.ld
+else
+    LDSCRIPT = flash-mcuboot-app.ld

Review comment:
       Either correct this name to `flash-mcu-app.ld` or rename a linker script

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-loader/defconfig
##########
@@ -0,0 +1,94 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARDCTL_ROMDISK=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LATE_INITIALIZE=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_NCHAINS=24
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_BOOTLOADER=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"
+CONFIG_MM_CIRCBUF=y
+CONFIG_MM_IOB=y
+CONFIG_MM_REGIONS=4
+CONFIG_MTD=y
+CONFIG_MTD_BYTE_WRITE=y
+CONFIG_MTD_PARTITION=y
+CONFIG_MTD_PROGMEM=y
+CONFIG_MTD_PROGMEM_ERASESTATE=y
+CONFIG_MTD_WRBUFFER=y
+CONFIG_NSH_ARCHINIT=y
+CONFIG_NSH_BUILTIN_APPS=y
+CONFIG_NSH_DISABLE_IFCONFIG=y
+CONFIG_NSH_DISABLE_IFUPDOWN=y
+CONFIG_NSH_DISABLE_PS=y
+CONFIG_NSH_FILEIOSIZE=512
+CONFIG_NSH_LINELEN=64
+CONFIG_NSH_MOTD=y
+CONFIG_NSH_MOTD_STRING="Wellcome to Nuttx MCUboot-Loader!"
+CONFIG_NSH_READLINE=y
+CONFIG_PREALLOC_TIMERS=4
+CONFIG_RAM_SIZE=245760
+CONFIG_RAM_START=0x20010000
+CONFIG_RAW_BINARY=y
+CONFIG_RR_INTERVAL=200
+CONFIG_SCHED_LPNTHREADS=4
+CONFIG_SCHED_LPWORK=y
+CONFIG_SCHED_WAITPID=y
+CONFIG_SDCLONE_DISABLE=y
+CONFIG_SIG_DEFAULT=y
+CONFIG_SIG_PREALLOC_IRQ_ACTIONS=10
+CONFIG_SPI=y
+CONFIG_START_DAY=6
+CONFIG_START_MONTH=12
+CONFIG_START_YEAR=2011

Review comment:
       maybe we can move to 2021 :)

##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/Make.defs
##########
@@ -22,7 +22,15 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-LDSCRIPT = flash.ld
+ifeq ($(CONFIG_APP_FORMAT_MCUBOOT),y)
+ifeq ($(CONFIG_MCUBOOT_BOOTLOADER),y)
+    LDSCRIPT = flash-mcuboot-loader.ld

Review comment:
       Either correct this name to `flash-mcu-loader.ld` or rename a linker script

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,141 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_ARM_MPU_EARLY_RESET=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_NET=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_ETH0_PHY_LAN8742A=y
+CONFIG_EXAMPLES_LEDS=y
+CONFIG_EXAMPLES_LEDS_LEDSET=0x03
+CONFIG_EXAMPLES_TCPECHO=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FS_ROMFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_I2C=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_THROTTLE=0
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_SLOT_CONFIRM_EXAMPLE=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"
+CONFIG_MMCSD=y
+CONFIG_MM_CIRCBUF=y
+CONFIG_MM_REGIONS=4
+CONFIG_MTD=y
+CONFIG_MTD_PARTITION=y
+CONFIG_MTD_PROGMEM=y

Review comment:
       This options are auto-enabled by `PROGMEM_OTA_PARTITION` that is enabled by `APP_FORMAT_MCUBOOT` and I do not see it enabled. Probably it is missing?

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,141 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_ARM_MPU_EARLY_RESET=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_NET=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_ETH0_PHY_LAN8742A=y
+CONFIG_EXAMPLES_LEDS=y
+CONFIG_EXAMPLES_LEDS_LEDSET=0x03
+CONFIG_EXAMPLES_TCPECHO=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FS_ROMFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_I2C=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_THROTTLE=0
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_SLOT_CONFIRM_EXAMPLE=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"
+CONFIG_MMCSD=y
+CONFIG_MM_CIRCBUF=y
+CONFIG_MM_REGIONS=4
+CONFIG_MTD=y
+CONFIG_MTD_PARTITION=y
+CONFIG_MTD_PROGMEM=y
+CONFIG_MTD_SECT512=y
+CONFIG_MTD_WRBUFFER=y
+CONFIG_NET=y
+CONFIG_NETDB_DNSCLIENT=y
+CONFIG_NETDB_DNSSERVER_IPv4ADDR=0x08080808
+CONFIG_NETDEV_PHY_IOCTL=y
+CONFIG_NETINIT_DRIPADDR=0xc0a80101
+CONFIG_NETINIT_IPADDR=0xc0a801e3
+CONFIG_NETINIT_NOMAC=y
+CONFIG_NETINIT_THREAD=y
+CONFIG_NETUTILS_DISCOVER=y
+CONFIG_NETUTILS_TELNETD=y
+CONFIG_NETUTILS_WEBCLIENT=y
+CONFIG_NETUTILS_WEBSERVER=y
+CONFIG_NET_BROADCAST=y
+CONFIG_NET_ETH_PKTSIZE=1500
+CONFIG_NET_ICMP=y
+CONFIG_NET_ICMP_SOCKET=y
+CONFIG_NET_IGMP=y
+CONFIG_NET_PKT=y
+CONFIG_NET_RECV_BUFSIZE=1500
+CONFIG_NET_ROUTE=y
+CONFIG_NET_SEND_BUFSIZE=1500
+CONFIG_NET_SOLINGER=y
+CONFIG_NET_STATISTICS=y
+CONFIG_NET_TCP=y
+CONFIG_NET_TCPBACKLOG=y
+CONFIG_NET_TCP_NPOLLWAITERS=4
+CONFIG_NET_TCP_WRITE_BUFFERS=y
+CONFIG_NET_UDP=y
+CONFIG_NET_UDP_CHECKSUMS=y
+CONFIG_NSH_ARCHINIT=y
+CONFIG_NSH_ARCHROMFS=y
+CONFIG_NSH_BUILTIN_APPS=y
+CONFIG_NSH_FILEIOSIZE=512
+CONFIG_NSH_LINELEN=64
+CONFIG_NSH_MMCSDSPIPORTNO=1
+CONFIG_NSH_MOTD=y
+CONFIG_NSH_MOTD_STRING="Wellcome to Nuttx MCUboot-Confirm from MCUboot-Loader!"

Review comment:
       ```suggestion
   CONFIG_NSH_MOTD_STRING="Wellcome to Nuttx MCUboot-Confirm from MCUboot-App!"
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-loader/defconfig
##########
@@ -0,0 +1,94 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARDCTL_ROMDISK=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LATE_INITIALIZE=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_NCHAINS=24
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_BOOTLOADER=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"
+CONFIG_MM_CIRCBUF=y
+CONFIG_MM_IOB=y
+CONFIG_MM_REGIONS=4
+CONFIG_MTD=y
+CONFIG_MTD_BYTE_WRITE=y
+CONFIG_MTD_PARTITION=y
+CONFIG_MTD_PROGMEM=y
+CONFIG_MTD_PROGMEM_ERASESTATE=y

Review comment:
       I do not see `APP_FORMAT_MCUBOOT` enabled. Is it missing?

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_boot_image.c
##########
@@ -0,0 +1,168 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_reset.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <debug.h>
+#include <stdio.h>
+#include <fcntl.h>
+
+#include <sys/boardctl.h>
+#include <nuttx/irq.h>
+#include <nuttx/cache.h>
+
+#include "nvic.h"
+#include "arm_arch.h"
+#include "barriers.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure represents the first two entries on NVIC vector table */
+
+struct arm_vector_table
+{
+  uint32_t spr;   /* Stack pointer on reset */
+  uint32_t reset; /* Pointer to reset exception handler */
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static void cleanup_arm_nvic(void);
+static void systick_disable(void);
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name:  cleanup_arm_nvic
+ *
+ * Description:
+ *   Acknowledge and disable all interrupts in NVIC
+ *
+ * Input Parameters:
+ *   None
+ *
+ *  Returned Value:
+ *    None
+ *
+ ****************************************************************************/
+
+static void cleanup_arm_nvic(void)
+{
+  int i;
+
+  /* Allow any pending interrupts to be recognized */
+
+  ARM_ISB();
+  cpsid();
+
+  /* Disable all interrupts */
+
+  for (i = 0; i < NR_IRQS; i += 32)
+    {
+      putreg32(0xffffffff, NVIC_IRQ_CLEAR(i));
+    }
+
+  /* Clear all pending interrupts */
+
+  for (i = 0; i < NR_IRQS; i += 32)
+    {
+      putreg32(0xffffffff, NVIC_IRQ_CLRPEND(i));
+    }
+}
+
+static void systick_disable(void)
+{
+  // interrupt disable
+  modifyreg32(NVIC_SYSTICK_CTRL, NVIC_SYSTICK_CTRL_TICKINT, 0);
+  modifyreg32(NVIC_SYSTICK_CTRL, NVIC_SYSTICK_CTRL_ENABLE, 0);
+
+  // counter disable
+	putreg32(0, NVIC_SYSTICK_CURRENT);
+}
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: board_boot_image
+ *
+ * Description:
+ *   This entry point is called by bootloader to jump to application image.
+ *
+ ****************************************************************************/
+
+int board_boot_image(FAR const char *path, uint32_t hdr_size)
+{
+  static struct arm_vector_table vt;
+  int fd;
+  ssize_t bytes;
+
+  fd = open(path, O_RDONLY | O_CLOEXEC);
+  if (fd < 0)
+    {
+      syslog(LOG_ERR, "Failed to open %s with: %d", path, fd);
+      return fd;
+    }
+
+  bytes = pread(fd, &vt, sizeof(vt), hdr_size);
+  if (bytes != sizeof(vt))
+    {
+      syslog(LOG_ERR, "Failed to read ARM vector table: %d", bytes);
+      return bytes < 0 ? bytes : -1;
+    }
+
+  cleanup_arm_nvic();
+
+#ifdef CONFIG_ARMV7M_DCACHE
+  up_disable_dcache();
+#endif
+#ifdef CONFIG_ARMV7M_ICACHE
+  up_disable_icache();
+#endif
+
+#ifdef CONFIG_ARM_MPU
+  mpu_control(false, false, false);
+#endif
+
+  systick_disable();
+
+  /* Set main and process stack pointers */
+
+  __asm__ __volatile__("\tmsr msp, %0\n" : : "r" (vt.spr));
+  setcontrol(0x00);
+  ARM_ISB();
+  ((void (*)(void))vt.reset)();
+
+  return 0;
+}

Review comment:
       ```suggestion
   }
   
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcu-app.ld
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/scripts/flash.ld
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The STM32H743ZI has 2048Kb of main FLASH memory. The flash memory is
+ * partitioned into a User Flash memory and a System Flash memory. Each
+ * of these memories has two banks:
+ *
+ *   1) User Flash memory:
+ *
+ *      Bank 1: Start address 0x0800:0000 to 0x080F:FFFF with 8 sectors, 128Kb each
+ *      Bank 2: Start address 0x0810:0000 to 0x081F:FFFF with 8 sectors, 128Kb each
+ *
+ *   2) System Flash memory:
+ *
+ *      Bank 1: Start address 0x1FF0:0000 to 0x1FF1:FFFF with 1 x 128Kb sector
+ *      Bank 1: Start address 0x1FF4:0000 to 0x1FF5:FFFF with 1 x 128Kb sector
+ *
+ *   3) User option bytes for user configuration, only in Bank 1.
+ *
+ * In the STM32H743ZI, two different boot spaces can be selected through
+ * the BOOT pin and the boot base address programmed in the BOOT_ADD0 and
+ * BOOT_ADD1 option bytes:
+ *
+ *   1) BOOT=0: Boot address defined by user option byte BOOT_ADD0[15:0].
+ *      ST programmed value: Flash memory at 0x0800:0000
+ *   2) BOOT=1: Boot address defined by user option byte BOOT_ADD1[15:0].
+ *      ST programmed value: System bootloader at 0x1FF0:0000
+ *
+ * TODO: Check next paragraph with nucleo schematics
+ *
+ * NuttX does not modify these option bytes. On the unmodified NUCLEO-H743ZI
+ * board, the BOOT0 pin is at ground so by default, the STM32 will boot
+ * to address 0x0800:0000 in FLASH.
+ *
+ * The STM32H743ZI also has 1024Kb of data SRAM.
+ * SRAM is split up into several blocks and into three power domains:
+ *
+ *   1) TCM SRAMs are dedicated to the Cortex-M7 and are accessible with
+ *      0 wait states by the Cortex-M7 and by MDMA through AHBS slave bus
+ *
+ *      1.1) 128Kb of DTCM-RAM beginning at address 0x2000:0000
+ *
+ *           The DTCM-RAM is organized as 2 x 64Kb DTCM-RAMs on 2 x 32 bit
+ *           DTCM ports. The DTCM-RAM could be used for critical real-time
+ *           data, such as interrupt service routines or stack / heap memory.
+ *           Both DTCM-RAMs can be used in parallel (for load/store operations)
+ *           thanks to the Cortex-M7 dual issue capability.
+ *
+ *      1.2)  64Kb of ITCM-RAM beginning at address 0x0000:0000
+ *
+ *           This RAM is connected to ITCM 64-bit interface designed for
+ *           execution of critical real-times routines by the CPU.
+ *
+ *   2) AXI SRAM (D1 domain) accessible by all system masters except BDMA
+ *      through D1 domain AXI bus matrix
+ *
+ *      2.1) 512Kb of SRAM beginning at address 0x2400:0000
+ *
+ *   3) AHB SRAM (D2 domain) accessible by all system masters except BDMA
+ *      through D2 domain AHB bus matrix
+ *
+ *      3.1) 128Kb of SRAM1 beginning at address 0x3000:0000
+ *      3.2) 128Kb of SRAM2 beginning at address 0x3002:0000
+ *      3.3)  32Kb of SRAM3 beginning at address 0x3004:0000
+ *
+ *      SRAM1 - SRAM3 are one contiguous block: 288Kb at address 0x3000:0000
+ *
+ *   4) AHB SRAM (D3 domain) accessible by most of system masters
+ *      through D3 domain AHB bus matrix
+ *
+ *      4.1)  64Kb of SRAM4 beginning at address 0x3800:0000
+ *      4.1)   4Kb of backup RAM beginning at address 0x3880:0000
+ *
+ * When booting from FLASH, FLASH memory is aliased to address 0x0000:0000
+ * where the code expects to begin execution by jumping to the entry point in
+ * the 0x0800:0000 address range.
+ */
+
+MEMORY
+{
+  itcm  (rwx) : ORIGIN = 0x00000000, LENGTH =   64K
+  flash (rx)  : ORIGIN = 0x08020000, LENGTH = 1920K

Review comment:
       Please see https://github.com/apache/incubator-nuttx/blob/5da9dbe57af2d9b0b22c6cf1b92e76dfe6dcb67b/boards/arm/samv7/same70-xplained/scripts/flash-dtcm-mcuboot-app.ld#L34 as a reference.
   The MCUboot compatible image requires a header to be prepended to image. The default header size is 32 bytes, but taking into account that NVIC start address should be aligned according to arch requirements the alignment might be bigger than 32.
   Also please align `LENGTH` with `config OTA_SLOT_SIZE` value
   

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,141 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_ARM_MPU_EARLY_RESET=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_NET=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_ETH0_PHY_LAN8742A=y
+CONFIG_EXAMPLES_LEDS=y
+CONFIG_EXAMPLES_LEDS_LEDSET=0x03
+CONFIG_EXAMPLES_TCPECHO=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FS_ROMFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_I2C=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_THROTTLE=0
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_SLOT_CONFIRM_EXAMPLE=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"

Review comment:
       ```suggestion
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcu-loader.ld
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/scripts/flash.ld

Review comment:
       ```suggestion
    * boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcu-loader.ld
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcu-app.ld
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/scripts/flash.ld

Review comment:
       ```suggestion
    * boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcu-app.ld
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/nucleo-h743zi.h
##########
@@ -36,10 +36,12 @@
 
 /* Configuration ************************************************************/
 
-#define HAVE_PROC       1
-#define HAVE_USBDEV     1
-#define HAVE_USBHOST    1
-#define HAVE_USBMONITOR 1
+#define HAVE_PROC            1
+#define HAVE_USBDEV          1
+#define HAVE_USBHOST         1
+#define HAVE_USBMONITOR      1
+#define HAVE_MTDCONFIG       1

Review comment:
       Please add below in this file
   ```
   #if !defined(CONFIG_MTD_CONFIG)
   #  undef HAVE_MTDCONFIG
   #endif
   
   /* On-chip Programming Memory */
   
   #if !defined(CONFIG_STM32H7_PROGMEM) || !defined(CONFIG_MTD_PROGMEM)
   #  undef HAVE_PROGMEM_CHARDEV
   #endif
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  //ASSERT((mtd_offset + mtd_size) <= up_progmem_neraseblocks() *
+  //        up_progmem_pagesize(0));
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+  {
+      return NULL;
+  }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+  {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+      {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+      }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+      {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+      }
+#endif
+  }
+
+  return ret;
+}
+#endif
+
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+
+
+  stm32h7_flash_unlock();  //TODO: Remove. Is only because cant link if removed.
+
+  /* Create an instance of the SAME70 FLASH program memory device driver */
+
+  g_progmem_mtd = progmem_initialize();
+  if (g_progmem_mtd == NULL)
+  {
+    return -EFAULT;
+  }
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+  ret = init_ota_partitions();
+  if (ret < 0)
+  {
+    return ret;
+  }
+#else
+  /* Use the FTL layer to wrap the MTD driver as a block driver */
+
+  ret = ftl_initialize(PROGMEM_MTD_MINOR, g_progmem_mtd);
+  if (ret < 0)
+  {
+    syslog(LOG_ERR, "ERROR: Failed to initialize the FTL layer: %d\n",
+          ret);
+    return ret;
+  }
+
+#ifdef CONFIG_BCH
+  char blockdev[18];
+  char chardev[12];
+
+  /* Use the minor number to create device paths */
+
+  snprintf(blockdev, 18, "/dev/mtdblock%d", PROGMEM_MTD_MINOR);
+  snprintf(chardev, 12, "/dev/mtd%d", PROGMEM_MTD_MINOR);
+
+  /* Now create a character device on the block device */
+
+  ret = bchdev_register(blockdev, chardev, false);
+  if (ret < 0)
+  {
+      syslog(LOG_ERR, "ERROR: bchdev_register %s failed: %d\n",
+             chardev, ret);
+      return ret;
+  }
+#endif /* CONFIG_BCH */
+#endif
+
+  return ret;
+}
+
+#endif /* HAVE_PROGMEM_CHARDEV */

Review comment:
       ```suggestion
   #endif /* HAVE_PROGMEM_CHARDEV */
   
   ```




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Yes. I think STM32_FORMAT_MCUBOOT and STM32_MCUBOOT_BOOTLOADER are fine.
   And yes, MCUboot options should be standardized in future across all the 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] xiaoxiang781216 commented on pull request #4908: nucleo-h743zi: add board files to support mcuboot

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


   @tito97sp could you squash the intermediate change into one patch?


-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  //ASSERT((mtd_offset + mtd_size) <= up_progmem_neraseblocks() *
+  //        up_progmem_pagesize(0));
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+  {
+      return NULL;
+  }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+  {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+      {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+      }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+      {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+      }
+#endif
+  }
+
+  return ret;
+}
+#endif
+
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+
+
+  stm32h7_flash_unlock();  //TODO: Remove. Is only because cant link if removed.
+
+  /* Create an instance of the SAME70 FLASH program memory device driver */

Review comment:
       ```suggestion
     /* Create an instance of the STM32H7FLASH program memory device driver */
   ```




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       https://github.com/apache/incubator-nuttx-apps/blob/master/boot/mcuboot/Kconfig:#L38 has same configuration option
   ```
   config MCUBOOT_BOOTLOADER
   	bool "MCUboot bootloader application"
   	default n
   	select BOARDCTL
   	select BOARDCTL_BOOT_IMAGE
   	---help---
   		MCUboot bootloader application
   ```
   I'm not sure if it will work with two configuration options with the same name




-- 
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] tito97sp commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       Yeah you are right. It does not seem to fail but I am gonna change the name to be cleaner. I would use `CONFIG_USE_MCUBOOT_BOOTLOADER`. 
   
   Any 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] pkarashchenko commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Maybe reasonable to rename `xxx-mcuboot-app.ld` to `xxx-mcuboot-ota.ld`?
   `CONFIG_APP_MCUBOOT_BOOTLOADER=y` should lead to picking `xxx-mcuboot-loader.ld` and `CONFIG_APP_MCUBOOT_BOOTLOADER=n` should pick other file. Probably  `xxx-mcuboot-ota.ld` name is better




-- 
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] tito97sp commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y

Review comment:
       I have problems with this configurations. It inverts the state of my MTD configurations.




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_APP_FORMAT_MCUBOOT

Review comment:
       ```suggestion
   if STM32_FORMAT_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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Or you can prepend `STM32H7` prefix, as an option

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Or you can prepend `STM32H7` prefix, as an option `CONFIG_STM32H7_MCUBOOT_BOOTLOADER` and `CONFIG_STM32H7_APP_FORMAT_MCUBOOT`

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Or you can prepend `STM32H7` prefix, as an option `CONFIG_STM32H7_MCUBOOT_BOOTLOADER` and `CONFIG_STM32H7_APP_FORMAT_MCUBOOT` and other MCUboot board specific configuration options

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Or just `STM32_` prefix like other option in same Kconfig like `STM32_ROMFS`




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  //ASSERT((mtd_offset + mtd_size) <= up_progmem_neraseblocks() *
+  //        up_progmem_pagesize(0));
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+  {
+      return NULL;
+  }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+  {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+      {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+      }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+      {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+      }
+#endif
+  }
+
+  return ret;
+}
+#endif
+
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+

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] pkarashchenko commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,141 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_ARM_MPU_EARLY_RESET=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_NET=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_ETH0_PHY_LAN8742A=y
+CONFIG_EXAMPLES_LEDS=y
+CONFIG_EXAMPLES_LEDS_LEDSET=0x03
+CONFIG_EXAMPLES_TCPECHO=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FS_ROMFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_I2C=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_THROTTLE=0
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_SLOT_CONFIRM_EXAMPLE=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"
+CONFIG_MMCSD=y
+CONFIG_MM_CIRCBUF=y
+CONFIG_MM_REGIONS=4
+CONFIG_MTD=y
+CONFIG_MTD_PARTITION=y
+CONFIG_MTD_PROGMEM=y
+CONFIG_MTD_SECT512=y
+CONFIG_MTD_WRBUFFER=y
+CONFIG_NET=y
+CONFIG_NETDB_DNSCLIENT=y
+CONFIG_NETDB_DNSSERVER_IPv4ADDR=0x08080808
+CONFIG_NETDEV_PHY_IOCTL=y
+CONFIG_NETINIT_DRIPADDR=0xc0a80101
+CONFIG_NETINIT_IPADDR=0xc0a801e3
+CONFIG_NETINIT_NOMAC=y
+CONFIG_NETINIT_THREAD=y
+CONFIG_NETUTILS_DISCOVER=y
+CONFIG_NETUTILS_TELNETD=y
+CONFIG_NETUTILS_WEBCLIENT=y
+CONFIG_NETUTILS_WEBSERVER=y
+CONFIG_NET_BROADCAST=y
+CONFIG_NET_ETH_PKTSIZE=1500
+CONFIG_NET_ICMP=y
+CONFIG_NET_ICMP_SOCKET=y
+CONFIG_NET_IGMP=y
+CONFIG_NET_PKT=y
+CONFIG_NET_RECV_BUFSIZE=1500
+CONFIG_NET_ROUTE=y
+CONFIG_NET_SEND_BUFSIZE=1500
+CONFIG_NET_SOLINGER=y
+CONFIG_NET_STATISTICS=y
+CONFIG_NET_TCP=y
+CONFIG_NET_TCPBACKLOG=y
+CONFIG_NET_TCP_NPOLLWAITERS=4
+CONFIG_NET_TCP_WRITE_BUFFERS=y
+CONFIG_NET_UDP=y
+CONFIG_NET_UDP_CHECKSUMS=y
+CONFIG_NSH_ARCHINIT=y
+CONFIG_NSH_ARCHROMFS=y
+CONFIG_NSH_BUILTIN_APPS=y
+CONFIG_NSH_FILEIOSIZE=512
+CONFIG_NSH_LINELEN=64
+CONFIG_NSH_MMCSDSPIPORTNO=1
+CONFIG_NSH_MOTD=y
+CONFIG_NSH_MOTD_STRING="Wellcome to Nuttx MCUboot-Confirm from MCUboot-Loader!"

Review comment:
       ```suggestion
   CONFIG_NSH_MOTD_STRING="Wellcome to Nuttx MCUboot-Confirm from MCUboot-App!"
   ```




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       https://github.com/apache/incubator-nuttx-apps/blob/master/boot/mcuboot/Kconfig:#L38..L44 has same configuration option
   ```
   config MCUBOOT_BOOTLOADER
   	bool "MCUboot bootloader application"
   	default n
   	select BOARDCTL
   	select BOARDCTL_BOOT_IMAGE
   	---help---
   		MCUboot bootloader application
   ```
   I'm not sure if it will work with two configuration options with the same name




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       The MCUboot in nuttx-apps has same configuration option: https://github.com/apache/incubator-nuttx-apps/blob/c9f736005ac3278faf67d6c9458101bff7882d46/boot/mcuboot/Kconfig#L38
   I'm not sure if it will work with two configuration options with the same name




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config OTA_SECONDARY_SLOT_OFFSET
+	hex "MCUboot application image secondary slot offset"
+	default "0x100000"

Review comment:
       Please check here. I see that `OTA_PRIMARY_SLOT_OFFSET`+`OTA_SLOT_SIZE` -> `0x20000 + 0xc0000 == 0xe0000` so there still is `0x20000` free room left

##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-loader/defconfig
##########
@@ -0,0 +1,94 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARDCTL_ROMDISK=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LATE_INITIALIZE=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_NCHAINS=24
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_BOOTLOADER=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"

Review comment:
       I think I fixed all warnings with https://github.com/mcu-tools/mcuboot/commit/7c890f4b075aed73e4c825ccf875b2fb9ebf2ded and that hash commit is the latest in nuttx-apps, see: https://github.com/apache/incubator-nuttx-apps/commit/16accc17bfe26956c52b9f886135dc5eb8aaf3b1




-- 
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] tito97sp commented on a change in pull request #4908: nucleo-h743zi: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcuboot-app.ld
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcuboot-app.ld
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The STM32H743ZI has 2048Kb of main FLASH memory. The flash memory is
+ * partitioned into a User Flash memory and a System Flash memory. Each
+ * of these memories has two banks:
+ *
+ *   1) User Flash memory:
+ *
+ *      Bank 1: Start address 0x0800:0000 to 0x080F:FFFF with 8 sectors, 128Kb each
+ *      Bank 2: Start address 0x0810:0000 to 0x081F:FFFF with 8 sectors, 128Kb each
+ *
+ *   2) System Flash memory:
+ *
+ *      Bank 1: Start address 0x1FF0:0000 to 0x1FF1:FFFF with 1 x 128Kb sector
+ *      Bank 1: Start address 0x1FF4:0000 to 0x1FF5:FFFF with 1 x 128Kb sector
+ *
+ *   3) User option bytes for user configuration, only in Bank 1.
+ *
+ * In the STM32H743ZI, two different boot spaces can be selected through
+ * the BOOT pin and the boot base address programmed in the BOOT_ADD0 and
+ * BOOT_ADD1 option bytes:
+ *
+ *   1) BOOT=0: Boot address defined by user option byte BOOT_ADD0[15:0].
+ *      ST programmed value: Flash memory at 0x0800:0000
+ *   2) BOOT=1: Boot address defined by user option byte BOOT_ADD1[15:0].
+ *      ST programmed value: System bootloader at 0x1FF0:0000
+ *
+ * TODO: Check next paragraph with nucleo schematics
+ *
+ * NuttX does not modify these option bytes. On the unmodified NUCLEO-H743ZI
+ * board, the BOOT0 pin is at ground so by default, the STM32 will boot
+ * to address 0x0800:0000 in FLASH.
+ *
+ * The STM32H743ZI also has 1024Kb of data SRAM.
+ * SRAM is split up into several blocks and into three power domains:
+ *
+ *   1) TCM SRAMs are dedicated to the Cortex-M7 and are accessible with
+ *      0 wait states by the Cortex-M7 and by MDMA through AHBS slave bus
+ *
+ *      1.1) 128Kb of DTCM-RAM beginning at address 0x2000:0000
+ *
+ *           The DTCM-RAM is organized as 2 x 64Kb DTCM-RAMs on 2 x 32 bit
+ *           DTCM ports. The DTCM-RAM could be used for critical real-time
+ *           data, such as interrupt service routines or stack / heap memory.
+ *           Both DTCM-RAMs can be used in parallel (for load/store operations)
+ *           thanks to the Cortex-M7 dual issue capability.
+ *
+ *      1.2)  64Kb of ITCM-RAM beginning at address 0x0000:0000
+ *
+ *           This RAM is connected to ITCM 64-bit interface designed for
+ *           execution of critical real-times routines by the CPU.
+ *
+ *   2) AXI SRAM (D1 domain) accessible by all system masters except BDMA
+ *      through D1 domain AXI bus matrix
+ *
+ *      2.1) 512Kb of SRAM beginning at address 0x2400:0000
+ *
+ *   3) AHB SRAM (D2 domain) accessible by all system masters except BDMA
+ *      through D2 domain AHB bus matrix
+ *
+ *      3.1) 128Kb of SRAM1 beginning at address 0x3000:0000
+ *      3.2) 128Kb of SRAM2 beginning at address 0x3002:0000
+ *      3.3)  32Kb of SRAM3 beginning at address 0x3004:0000
+ *
+ *      SRAM1 - SRAM3 are one contiguous block: 288Kb at address 0x3000:0000
+ *
+ *   4) AHB SRAM (D3 domain) accessible by most of system masters
+ *      through D3 domain AHB bus matrix
+ *
+ *      4.1)  64Kb of SRAM4 beginning at address 0x3800:0000
+ *      4.1)   4Kb of backup RAM beginning at address 0x3880:0000
+ *
+ * When booting from FLASH, FLASH memory is aliased to address 0x0000:0000
+ * where the code expects to begin execution by jumping to the entry point in
+ * the 0x0800:0000 address range.
+ */
+
+MEMORY
+{
+  itcm  (rwx) : ORIGIN = 0x00000000, LENGTH =   64K
+  flash (rx)  : ORIGIN = 0x08020000, LENGTH = 1920K

Review comment:
       I am becoming aware that image start should be at address 0x08020200, as the second slot of the first bank starts at 0x08020000 and the mcuboot header size is intended to be 500 bytes (0x200).




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/Make.defs
##########
@@ -22,7 +22,15 @@ include $(TOPDIR)/.config
 include $(TOPDIR)/tools/Config.mk
 include $(TOPDIR)/arch/arm/src/armv7-m/Toolchain.defs
 
-LDSCRIPT = flash.ld
+ifeq ($(CONFIG_APP_FORMAT_MCUBOOT),y)
+ifeq ($(CONFIG_USE_MCUBOOT_BOOTLOADER),y)
+    LDSCRIPT = flash-mcuboot-loader.ld
+else
+    LDSCRIPT = flash-mcuboot-app.ld
+endif
+else
+  LDSCRIPT = flash.ld
+endif

Review comment:
       ```suggestion
   ifeq ($(CONFIG_APP_FORMAT_MCUBOOT),y)
     ifeq ($(CONFIG_USE_MCUBOOT_BOOTLOADER),y)
       LDSCRIPT = flash-mcuboot-loader.ld
     else
       LDSCRIPT = flash-mcuboot-app.ld
     endif
   else
     LDSCRIPT = flash.ld
   endif
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+

Review comment:
       ```suggestion
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcu-loader.ld
##########
@@ -0,0 +1,200 @@
+/****************************************************************************

Review comment:
       Please remove duplication. boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcuboot-loader.ld is already available




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_FORMAT_MCUBOOT
+
+config STM32_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH

Review comment:
       ```suggestion
   config STM32_OTA_PRIMARY_SLOT_DEVPATH
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_FORMAT_MCUBOOT
+
+config STM32_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config OTA_SECONDARY_SLOT_DEVPATH

Review comment:
       ```suggestion
   config STM32_OTA_SECONDARY_SLOT_DEVPATH
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_FORMAT_MCUBOOT
+
+config STM32_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config OTA_SCRATCH_DEVPATH

Review comment:
       ```suggestion
   config STM32_OTA_SCRATCH_DEVPATH
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_FORMAT_MCUBOOT
+
+config STM32_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config OTA_PRIMARY_SLOT_OFFSET

Review comment:
       ```suggestion
   config STM32_OTA_PRIMARY_SLOT_OFFSET
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if STM32_FORMAT_MCUBOOT
+
+config STM32_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.
+
+comment "MCUboot Application Image OTA Update support"
+
+config OTA_PRIMARY_SLOT_DEVPATH
+	string "Application image primary slot device path"
+	default "/dev/ota0"
+
+config OTA_SECONDARY_SLOT_DEVPATH
+	string "Application image secondary slot device path"
+	default "/dev/ota1"
+
+config OTA_SCRATCH_DEVPATH
+	string "Scratch partition device path"
+	default "/dev/otascratch"
+
+config OTA_PRIMARY_SLOT_OFFSET
+	hex "MCUboot application image primary slot offset"
+	default "0x20000"
+
+config OTA_SECONDARY_SLOT_OFFSET

Review comment:
       ```suggestion
   config STM32_OTA_SECONDARY_SLOT_OFFSET
   ```




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Maybe also reasonable to remove `APP_` from `APP_FORMAT_MCUBOOT` and rename to `STM32_FORMAT_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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       https://github.com/apache/incubator-nuttx-apps/blob/master/boot/mcuboot/Kconfig:L38 has same configuration option
   ```
   config MCUBOOT_BOOTLOADER
   	bool "MCUboot bootloader application"
   	default n
   	select BOARDCTL
   	select BOARDCTL_BOOT_IMAGE
   	---help---
   		MCUboot bootloader application
   ```
   I'm not sure if it will work with two configuration options with the same name




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  //ASSERT((mtd_offset + mtd_size) <= up_progmem_neraseblocks() *
+  //        up_progmem_pagesize(0));
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+  {
+      return NULL;
+  }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+  {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+      {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+      }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+      {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+      }
+#endif
+  }
+
+  return ret;
+}
+#endif
+
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+
+
+  stm32h7_flash_unlock();  //TODO: Remove. Is only because cant link if removed.
+
+  /* Create an instance of the SAME70 FLASH program memory device driver */

Review comment:
       ```suggestion
     /* Create an instance of the STM32H7 FLASH program memory device driver */
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y

Review comment:
       Please consider if this can be defaulted to `n`.

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y

Review comment:
       Please consider if this can be defaulted to `n`.




-- 
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 pull request #4908: nucleo-h743zi: add board files to support mcuboot

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


   Hi @tito97sp ,
   
   I strong recommend you organize code like #4981. That way almost enable to use MCUboot on any STM32H7 board, in a easy way. Otherwise people will start to duplicate your code, which is complete unnecessary.
   
   My concerns around MCUboot is that starting thinking support only one board seems to be the easy way but is hard to maintain. I would like encourage you to create the common folder and put all necessary things there. Then, only enable support in your board.


-- 
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 commented on pull request #4908: nucleo-h743zi: add board files to support mcuboot

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


   > @acassis @pkarashchenko is this patch ready for merge?
   
   It seams he fixed all the suggestions, but the CI is failing. Maybe @tito97sp needs to rebase it to the HEAD to fix these issues.


-- 
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 #4908: nucleo-h743zi: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include "nucleo-h743zi.h"
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_STM32_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_STM32_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_STM32_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_STM32_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                               uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);

Review comment:
       Why
   ```
    
    ​  ​ASSERT​((mtd_offset + mtd_size) <= ​up_progmem_neraseblocks​() * 
    ​          ​up_progmem_pagesize​(​0​));
   ```
   has been removed? Is it failing on this target?

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include "nucleo-h743zi.h"
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_STM32_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_STM32_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_STM32_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_STM32_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                               uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+    {
+      return NULL;
+    }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+    {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+        {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+        }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+        {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+        }
+#endif
+    }
+
+  return ret;
+}
+#endif /* CONFIG_STM32_PROGMEM_OTA_PARTITION */
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+
+  stm32h7_flash_unlock();

Review comment:
       I see that flash lock/unlock is managed by stm32h7 progmem layer. Why do we need this call here?




-- 
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 pull request #4908: nucleo-h743zi: add board files to support mcuboot

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


   @acassis @pkarashchenko is this patch ready for merge?


-- 
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] tito97sp commented on a change in pull request #4908: nucleo-h743zi: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION

Review comment:
       Done

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_bringup.c
##########
@@ -309,7 +309,7 @@ int stm32_bringup(void)
   ret = cdcacm_initialize(0, NULL);
   if (ret < 0)
     {
-      syslog(LOG_ERR, "ERROR: cdcacm_initialize failed: %d\n", ret);
+        syslog(LOG_ERR, "ERROR: cdcacm_initialize failed: %d\n", ret);

Review comment:
       ok

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_bringup.c
##########
@@ -323,5 +323,16 @@ int stm32_bringup(void)
     }
 #endif
 
+#ifdef CONFIG_MTD
+#ifdef HAVE_PROGMEM_CHARDEV
+  ret = stm32_progmem_init();
+  if (ret < 0)
+    {
+      syslog(LOG_ERR, "ERROR: Failed to initialize MTD progmem: %d\n",
+              ret);

Review comment:
       ok.




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       ```suggestion
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config APP_MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default n
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address. 

Review comment:
       ```suggestion
   		built to another entry point address.
   ```




-- 
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] tito97sp commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-loader/defconfig
##########
@@ -0,0 +1,94 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_ARCH="arm"
+CONFIG_ARCH_BOARD="nucleo-h743zi"
+CONFIG_ARCH_BOARD_NUCLEO_H743ZI=y
+CONFIG_ARCH_CHIP="stm32h7"
+CONFIG_ARCH_CHIP_STM32H743ZI=y
+CONFIG_ARCH_CHIP_STM32H7=y
+CONFIG_ARCH_STACKDUMP=y
+CONFIG_ARMV7M_BASEPRI_WAR=y
+CONFIG_ARMV7M_DCACHE=y
+CONFIG_ARMV7M_DTCM=y
+CONFIG_ARMV7M_ICACHE=y
+CONFIG_ARMV7M_MEMCPY=y
+CONFIG_ARMV7M_STACKCHECK=y
+CONFIG_ARMV7M_USEBASEPRI=y
+CONFIG_BOARDCTL_RESET=y
+CONFIG_BOARDCTL_ROMDISK=y
+CONFIG_BOARD_CUSTOM_LEDS=y
+CONFIG_BOARD_LATE_INITIALIZE=y
+CONFIG_BOARD_LOOPSPERMSEC=43103
+CONFIG_BOOT_MCUBOOT=y
+CONFIG_BUILTIN=y
+CONFIG_CLOCK_MONOTONIC=y
+CONFIG_DEBUG_FEATURES=y
+CONFIG_DEBUG_IRQ=y
+CONFIG_DEBUG_SYMBOLS=y
+CONFIG_DRVR_READAHEAD=y
+CONFIG_DRVR_WRITEBUFFER=y
+CONFIG_EXPERIMENTAL=y
+CONFIG_FAT_LCNAMES=y
+CONFIG_FAT_LFN=y
+CONFIG_FS_FAT=y
+CONFIG_FS_PROCFS=y
+CONFIG_FTL_WRITEBUFFER=y
+CONFIG_HAVE_CXX=y
+CONFIG_HAVE_CXXINITIALIZE=y
+CONFIG_IOB_NBUFFERS=24
+CONFIG_IOB_NCHAINS=24
+CONFIG_LIBC_LZF=y
+CONFIG_LIBM=y
+CONFIG_MCUBOOT_BOOTLOADER=y
+CONFIG_MCUBOOT_VERSION="ecd34c116873bf187a3d7431151249ec856738d0"
+CONFIG_MM_CIRCBUF=y
+CONFIG_MM_IOB=y
+CONFIG_MM_REGIONS=4
+CONFIG_MTD=y
+CONFIG_MTD_BYTE_WRITE=y
+CONFIG_MTD_PARTITION=y
+CONFIG_MTD_PROGMEM=y
+CONFIG_MTD_PROGMEM_ERASESTATE=y
+CONFIG_MTD_WRBUFFER=y
+CONFIG_NSH_ARCHINIT=y
+CONFIG_NSH_BUILTIN_APPS=y
+CONFIG_NSH_DISABLE_IFCONFIG=y
+CONFIG_NSH_DISABLE_IFUPDOWN=y
+CONFIG_NSH_DISABLE_PS=y
+CONFIG_NSH_FILEIOSIZE=512
+CONFIG_NSH_LINELEN=64
+CONFIG_NSH_MOTD=y
+CONFIG_NSH_MOTD_STRING="Wellcome to Nuttx MCUboot-Loader!"
+CONFIG_NSH_READLINE=y
+CONFIG_PREALLOC_TIMERS=4
+CONFIG_RAM_SIZE=245760
+CONFIG_RAM_START=0x20010000
+CONFIG_RAW_BINARY=y
+CONFIG_RR_INTERVAL=200
+CONFIG_SCHED_LPNTHREADS=4
+CONFIG_SCHED_LPWORK=y
+CONFIG_SCHED_WAITPID=y
+CONFIG_SDCLONE_DISABLE=y
+CONFIG_SIG_DEFAULT=y
+CONFIG_SIG_PREALLOC_IRQ_ACTIONS=10
+CONFIG_SPI=y
+CONFIG_START_DAY=6
+CONFIG_START_MONTH=12
+CONFIG_START_YEAR=2011

Review comment:
       Totally agree. Although 2011 was a great year ;)
   




-- 
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 #4908: add board files to support mcuboot

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


   > When selected PROGMEM_OTA_PARTITION by STM32_FORMAT_MCUBOOT it inverts the values of the next FLAGS: MTD MTD_BYTE_WRITE MTD_PARTITION MTD_PROGMEM MTD_PROGMEM_ERASESTATE
   > 
   > If my configuration already had activated the MTD functionalit, when using the PROGMEM_OTA_PARTITION it becomes false and opossite if it was not activated.
   > 
   > Do you know why it is this happening?? I have it selected as default = n as suggested previously
   
   I do not fully understand what do you mean. Enabling `PROGMEM_OTA_PARTITION` should enable `MTD`, `MTD_BYTE_WRITE`, `MTD_PARTITION`, `MTD_PROGMEM` and `MTD_PROGMEM_ERASESTATE`. Because enabling is implicit it those will no longer reside in `defconfig` file, but will be generated correctly into `.config` and that is ok. If this does not answer your question you can email me to petro.karashchenko@gmail.com with `make menuconfig` screenshots and defconfig file, so I can make a further 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 a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       I would suggest `CONFIG_APP_MCUBOOT_BOOTLOADER`




-- 
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] tito97sp commented on pull request #4908: add board files to support mcuboot

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


   When selected PROGMEM_OTA_PARTITION by STM32_FORMAT_MCUBOOT it inverts the values of the next FLAGS:
           MTD
   	MTD_BYTE_WRITE
   	MTD_PARTITION
   	MTD_PROGMEM
   	MTD_PROGMEM_ERASESTATE
   
   If my configuration already had activated the MTD functionalit, when using the PROGMEM_OTA_PARTITION it becomes false and opossite if it was not activated.
   
   Do you know why it is this happening?? I have it selected as default = n as suggested previously
   


-- 
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 #4908: nucleo-h743zi: add board files to support mcuboot

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


   @tito97sp do you have any plans when you can update this 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] pkarashchenko commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)

Review comment:
       ```suggestion
                                                  uint32_t mtd_size)
   ```




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       https://github.com/apache/incubator-nuttx-apps/blob/c9f736005ac3278faf67d6c9458101bff7882d46/boot/mcuboot/Kconfig#L38..L44 has same configuration option
   ```
   config MCUBOOT_BOOTLOADER
   	bool "MCUboot bootloader application"
   	default n
   	select BOARDCTL
   	select BOARDCTL_BOOT_IMAGE
   	---help---
   		MCUboot bootloader application
   ```
   I'm not sure if it will work with two configuration options with the same name




-- 
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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default y
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig APP_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default y
+	select PROGMEM_OTA_PARTITION
+	---help---
+		The MCUboot support of loading the firmware images.
+
+if APP_FORMAT_MCUBOOT
+
+config MCUBOOT_BOOTLOADER
+	bool "MCUboot bootloader application"
+	default y
+	---help---
+		This switch between linker scripts to allow an application be
+		built to another entry point address.

Review comment:
       https://github.com/apache/incubator-nuttx-apps/blob/master/boot/mcuboot/Kconfig#L38 has same configuration option
   ```
   config MCUBOOT_BOOTLOADER
   	bool "MCUboot bootloader application"
   	default n
   	select BOARDCTL
   	select BOARDCTL_BOOT_IMAGE
   	---help---
   		MCUboot bootloader application
   ```
   I'm not sure if it will work with two configuration options with the same name




-- 
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] tito97sp commented on a change in pull request #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_boot_image.c
##########
@@ -0,0 +1,168 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_reset.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <debug.h>
+#include <stdio.h>
+#include <fcntl.h>
+
+#include <sys/boardctl.h>
+#include <nuttx/irq.h>
+#include <nuttx/cache.h>
+
+#include "nvic.h"
+#include "arm_arch.h"
+#include "barriers.h"
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+/* This structure represents the first two entries on NVIC vector table */
+
+struct arm_vector_table
+{
+  uint32_t spr;   /* Stack pointer on reset */
+  uint32_t reset; /* Pointer to reset exception handler */
+};
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+static void cleanup_arm_nvic(void);
+static void systick_disable(void);
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name:  cleanup_arm_nvic
+ *
+ * Description:
+ *   Acknowledge and disable all interrupts in NVIC
+ *
+ * Input Parameters:
+ *   None
+ *
+ *  Returned Value:
+ *    None
+ *
+ ****************************************************************************/
+
+static void cleanup_arm_nvic(void)
+{
+  int i;
+
+  /* Allow any pending interrupts to be recognized */
+
+  ARM_ISB();
+  cpsid();
+
+  /* Disable all interrupts */
+
+  for (i = 0; i < NR_IRQS; i += 32)
+    {
+      putreg32(0xffffffff, NVIC_IRQ_CLEAR(i));
+    }
+
+  /* Clear all pending interrupts */
+
+  for (i = 0; i < NR_IRQS; i += 32)
+    {
+      putreg32(0xffffffff, NVIC_IRQ_CLRPEND(i));
+    }
+}
+
+static void systick_disable(void)

Review comment:
       Sure. I did not know you have already done it. 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.

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 #4908: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/configs/mcuboot-app/defconfig
##########
@@ -0,0 +1,142 @@
+#
+# This file is autogenerated: PLEASE DO NOT EDIT IT.
+#
+# You can use "make menuconfig" to make any modifications to the installed .config file.
+# You can then do "make savedefconfig" to generate a new defconfig file that includes your
+# modifications.
+#
+# CONFIG_SPI_EXCHANGE is not set
+# CONFIG_STM32H7_SYSCFG is not set
+CONFIG_APP_FORMAT_MCUBOOT=y
+CONFIG_APP_MCUBOOT_BOOTLOADER=y

Review comment:
       Or just `STM32_` prefix (CONFIG_STM32_MCUBOOT_BOOTLOADER) like other option in same Kconfig like `STM32_ROMFS`




-- 
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 #4908: nucleo-h743zi: add board files to support mcuboot

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



##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION

Review comment:
       ```suggestion
   config STM32_PROGMEM_OTA_PARTITION
   ```
   

##########
File path: boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcuboot-app.ld
##########
@@ -0,0 +1,200 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/scripts/flash-mcuboot-app.ld
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/* The STM32H743ZI has 2048Kb of main FLASH memory. The flash memory is
+ * partitioned into a User Flash memory and a System Flash memory. Each
+ * of these memories has two banks:
+ *
+ *   1) User Flash memory:
+ *
+ *      Bank 1: Start address 0x0800:0000 to 0x080F:FFFF with 8 sectors, 128Kb each
+ *      Bank 2: Start address 0x0810:0000 to 0x081F:FFFF with 8 sectors, 128Kb each
+ *
+ *   2) System Flash memory:
+ *
+ *      Bank 1: Start address 0x1FF0:0000 to 0x1FF1:FFFF with 1 x 128Kb sector
+ *      Bank 1: Start address 0x1FF4:0000 to 0x1FF5:FFFF with 1 x 128Kb sector
+ *
+ *   3) User option bytes for user configuration, only in Bank 1.
+ *
+ * In the STM32H743ZI, two different boot spaces can be selected through
+ * the BOOT pin and the boot base address programmed in the BOOT_ADD0 and
+ * BOOT_ADD1 option bytes:
+ *
+ *   1) BOOT=0: Boot address defined by user option byte BOOT_ADD0[15:0].
+ *      ST programmed value: Flash memory at 0x0800:0000
+ *   2) BOOT=1: Boot address defined by user option byte BOOT_ADD1[15:0].
+ *      ST programmed value: System bootloader at 0x1FF0:0000
+ *
+ * TODO: Check next paragraph with nucleo schematics
+ *
+ * NuttX does not modify these option bytes. On the unmodified NUCLEO-H743ZI
+ * board, the BOOT0 pin is at ground so by default, the STM32 will boot
+ * to address 0x0800:0000 in FLASH.
+ *
+ * The STM32H743ZI also has 1024Kb of data SRAM.
+ * SRAM is split up into several blocks and into three power domains:
+ *
+ *   1) TCM SRAMs are dedicated to the Cortex-M7 and are accessible with
+ *      0 wait states by the Cortex-M7 and by MDMA through AHBS slave bus
+ *
+ *      1.1) 128Kb of DTCM-RAM beginning at address 0x2000:0000
+ *
+ *           The DTCM-RAM is organized as 2 x 64Kb DTCM-RAMs on 2 x 32 bit
+ *           DTCM ports. The DTCM-RAM could be used for critical real-time
+ *           data, such as interrupt service routines or stack / heap memory.
+ *           Both DTCM-RAMs can be used in parallel (for load/store operations)
+ *           thanks to the Cortex-M7 dual issue capability.
+ *
+ *      1.2)  64Kb of ITCM-RAM beginning at address 0x0000:0000
+ *
+ *           This RAM is connected to ITCM 64-bit interface designed for
+ *           execution of critical real-times routines by the CPU.
+ *
+ *   2) AXI SRAM (D1 domain) accessible by all system masters except BDMA
+ *      through D1 domain AXI bus matrix
+ *
+ *      2.1) 512Kb of SRAM beginning at address 0x2400:0000
+ *
+ *   3) AHB SRAM (D2 domain) accessible by all system masters except BDMA
+ *      through D2 domain AHB bus matrix
+ *
+ *      3.1) 128Kb of SRAM1 beginning at address 0x3000:0000
+ *      3.2) 128Kb of SRAM2 beginning at address 0x3002:0000
+ *      3.3)  32Kb of SRAM3 beginning at address 0x3004:0000
+ *
+ *      SRAM1 - SRAM3 are one contiguous block: 288Kb at address 0x3000:0000
+ *
+ *   4) AHB SRAM (D3 domain) accessible by most of system masters
+ *      through D3 domain AHB bus matrix
+ *
+ *      4.1)  64Kb of SRAM4 beginning at address 0x3800:0000
+ *      4.1)   4Kb of backup RAM beginning at address 0x3880:0000
+ *
+ * When booting from FLASH, FLASH memory is aliased to address 0x0000:0000
+ * where the code expects to begin execution by jumping to the entry point in
+ * the 0x0800:0000 address range.
+ */
+
+MEMORY
+{
+  itcm  (rwx) : ORIGIN = 0x00000000, LENGTH =   64K
+  flash (rx)  : ORIGIN = 0x08020000, LENGTH = 1920K

Review comment:
       `ORIGIN = 0x08020000` still seems to be strange to me. I'm not sure what MCUboot header size is used with this example and how NVIC start address alignment requirements are satisfied?

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_bringup.c
##########
@@ -309,7 +309,7 @@ int stm32_bringup(void)
   ret = cdcacm_initialize(0, NULL);
   if (ret < 0)
     {
-      syslog(LOG_ERR, "ERROR: cdcacm_initialize failed: %d\n", ret);
+        syslog(LOG_ERR, "ERROR: cdcacm_initialize failed: %d\n", ret);

Review comment:
       ```suggestion
         syslog(LOG_ERR, "ERROR: cdcacm_initialize failed: %d\n", ret);
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE
+
+menuconfig STM32_FORMAT_MCUBOOT
+	bool "MCUboot-bootable format"
+	default n
+	select PROGMEM_OTA_PARTITION

Review comment:
       ```suggestion
   	select STM32_PROGMEM_OTA_PARTITION
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include "nucleo-h743zi.h"
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_STM32_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_STM32_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_STM32_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                               uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+    {
+      return NULL;
+    }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+    {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+        {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+        }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+        {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+        }
+#endif
+    }
+
+  return ret;
+}
+#endif
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+
+  stm32h7_flash_unlock();
+
+  /* Create an instance of the FLASH program memory device driver */
+
+  g_progmem_mtd = progmem_initialize();
+  if (g_progmem_mtd == NULL)
+    {
+      return -EFAULT;
+    }
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+  ret = init_ota_partitions();
+  if (ret < 0)
+    {
+      return ret;
+    }
+#else
+  /* Use the FTL layer to wrap the MTD driver as a block driver */
+
+  ret = ftl_initialize(PROGMEM_MTD_MINOR, g_progmem_mtd);
+  if (ret < 0)
+    {
+      syslog(LOG_ERR, "ERROR: Failed to initialize the FTL layer: %d\n",
+            ret);

Review comment:
       ```suggestion
                ret);
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/Kconfig
##########
@@ -27,4 +27,66 @@ config STM32_ROMFS_IMAGEFILE
         depends on STM32_ROMFS
         default "../../../rom.img"
 
+
+config PROGMEM_OTA_PARTITION
+	bool "Progmem ota partitions"
+	default n
+	select MTD
+	select MTD_BYTE_WRITE
+	select MTD_PARTITION
+	select MTD_PROGMEM
+	select MTD_PROGMEM_ERASESTATE

Review comment:
       ```suggestion
   	select MTD_PROGMEM_ERASESTATE
   	select STM32H7_PROGMEM
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/nucleo-h743zi.h
##########
@@ -78,6 +80,21 @@
 #  undef HAVE_USBMONITOR
 #endif
 
+/* On-chip Programming Memory */
+
+#if !defined(CONFIG_STM32H7_PROGMEM) || !defined(CONFIG_MTD_PROGMEM)
+#  undef HAVE_PROGMEM_CHARDEV
+#endif
+
+/* This is the on-chip progmem memory driver minor number */
+
+#define PROGMEM_MTD_MINOR 0
+
+/* flash  */
+#if defined(CONFIG_MMCSD)
+#   define FLASH_BASED_PARAMS

Review comment:
       ```suggestion
   #  define FLASH_BASED_PARAMS
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/nucleo-h743zi.h
##########
@@ -78,6 +80,21 @@
 #  undef HAVE_USBMONITOR
 #endif
 
+/* On-chip Programming Memory */

Review comment:
       ```suggestion
   #if !defined(CONFIG_MTD_CONFIG)
   #  undef HAVE_MTDCONFIG
   #endif
   
   /* On-chip Programming Memory */
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include "nucleo-h743zi.h"
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)

Review comment:
       ```suggestion
   #if defined(CONFIG_STM32_PROGMEM_OTA_PARTITION)
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_bringup.c
##########
@@ -323,5 +323,16 @@ int stm32_bringup(void)
     }
 #endif
 
+#ifdef CONFIG_MTD
+#ifdef HAVE_PROGMEM_CHARDEV
+  ret = stm32_progmem_init();
+  if (ret < 0)
+    {
+      syslog(LOG_ERR, "ERROR: Failed to initialize MTD progmem: %d\n",
+              ret);

Review comment:
       ```suggestion
         syslog(LOG_ERR, "ERROR: Failed to initialize MTD progmem: %d\n", ret);
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_bringup.c
##########
@@ -296,7 +296,7 @@ int stm32_bringup(void)
   ret = stm32_wlinitialize();
   if (ret < 0)
     {
-      syslog(LOG_ERR, "ERROR: Failed to initialize wireless driver: %d\n",
+        syslog(LOG_ERR, "ERROR: Failed to initialize wireless driver: %d\n",

Review comment:
       ```suggestion
         syslog(LOG_ERR, "ERROR: Failed to initialize wireless driver: %d\n",
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,279 @@
+/****************************************************************************
+ * boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include "nucleo-h743zi.h"
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_STM32_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_STM32_OTA_SLOT_SIZE,
+    .devpath = CONFIG_STM32_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_STM32_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_STM32_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_STM32_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                               uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+    {
+      return NULL;
+    }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+    {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+        {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+        }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+        {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+        }
+#endif
+    }
+
+  return ret;
+}
+#endif
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+
+  stm32h7_flash_unlock();
+
+  /* Create an instance of the FLASH program memory device driver */
+
+  g_progmem_mtd = progmem_initialize();
+  if (g_progmem_mtd == NULL)
+    {
+      return -EFAULT;
+    }
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+  ret = init_ota_partitions();
+  if (ret < 0)
+    {
+      return ret;
+    }
+#else
+  /* Use the FTL layer to wrap the MTD driver as a block driver */
+
+  ret = ftl_initialize(PROGMEM_MTD_MINOR, g_progmem_mtd);
+  if (ret < 0)
+    {
+      syslog(LOG_ERR, "ERROR: Failed to initialize the FTL layer: %d\n",
+            ret);
+      return ret;
+    }
+
+#ifdef CONFIG_BCH
+  char blockdev[18];
+  char chardev[12];
+
+  /* Use the minor number to create device paths */
+
+  snprintf(blockdev, 18, "/dev/mtdblock%d", PROGMEM_MTD_MINOR);
+  snprintf(chardev, 12, "/dev/mtd%d", PROGMEM_MTD_MINOR);
+
+  /* Now create a character device on the block device */
+
+  ret = bchdev_register(blockdev, chardev, false);
+  if (ret < 0)
+    {
+      syslog(LOG_ERR, "ERROR: bchdev_register %s failed: %d\n",
+              chardev, ret);

Review comment:
       ```suggestion
                chardev, ret);
   ```

##########
File path: boards/arm/stm32h7/nucleo-h743zi/src/stm32_progmem.c
##########
@@ -0,0 +1,284 @@
+/****************************************************************************
+ * apps/examples/mtdpart/stm32_progmem.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/mount.h>
+
+#include <stdbool.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+
+#include <nuttx/progmem.h>
+#include <nuttx/drivers/drivers.h>
+#include <nuttx/fs/ioctl.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include <stm32.h>
+#include <nucleo-h743zi.h>
+#include <stm32_flash.h>
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x)                (sizeof((x)) / sizeof((x)[0]))
+
+/* Configuration ************************************************************/
+
+/* Make sure that support for MTD partitions is enabled */
+#ifdef CONFIG_MTD
+
+#ifndef CONFIG_MTD_PARTITION
+#  error "CONFIG_MTD_PARTITION is required"
+#endif
+
+#endif
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+struct ota_partition_s
+{
+  uint32_t    offset;          /* Partition offset from the beginning of MTD */
+  uint32_t    size;            /* Partition size in bytes */
+  const char *devpath;         /* Partition device path */
+};
+
+#endif
+
+/****************************************************************************
+ * Private Function Prototypes
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size);
+static int init_ota_partitions(void);
+#endif
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+static FAR struct mtd_dev_s *g_progmem_mtd;
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+  {
+    .offset  = CONFIG_OTA_PRIMARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_PRIMARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SECONDARY_SLOT_OFFSET,
+    .size    = CONFIG_OTA_SLOT_SIZE,
+    .devpath = CONFIG_OTA_SECONDARY_SLOT_DEVPATH
+  },
+  {
+    .offset  = CONFIG_OTA_SCRATCH_OFFSET,
+    .size    = CONFIG_OTA_SCRATCH_SIZE,
+    .devpath = CONFIG_OTA_SCRATCH_DEVPATH
+  }
+};
+#endif
+
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+
+/****************************************************************************
+ * Name: sam_progmem_alloc_mtdpart
+ *
+ * Description:
+ *   Allocate an MTD partition from FLASH.
+ *
+ * Input Parameters:
+ *   mtd_offset - MTD Partition offset from the base address in FLASH.
+ *   mtd_size   - Size for the MTD partition.
+ *
+ * Returned Value:
+ *   MTD partition data pointer on success, NULL on failure.
+ *
+ ****************************************************************************/
+
+static struct mtd_dev_s *progmem_alloc_mtdpart(uint32_t mtd_offset,
+                                                   uint32_t mtd_size)
+{
+  uint32_t blocks;
+  ssize_t startblock;
+
+  //ASSERT((mtd_offset + mtd_size) <= up_progmem_neraseblocks() *
+  //        up_progmem_pagesize(0));
+  ASSERT((mtd_offset % up_progmem_pagesize(0)) == 0);
+  ASSERT((mtd_size % up_progmem_pagesize(0)) == 0);
+
+  finfo("\tMTD offset = 0x%"PRIx32"\n", mtd_offset);
+  finfo("\tMTD size = 0x%"PRIx32"\n", mtd_size);
+
+  startblock = up_progmem_getpage(mtd_offset + up_progmem_getaddress(0));
+  if (startblock < 0)
+  {
+      return NULL;
+  }
+
+  blocks = mtd_size / up_progmem_pagesize(0);
+
+  return mtd_partition(g_progmem_mtd, startblock, blocks);
+}
+
+/****************************************************************************
+ * Name: init_ota_partitions
+ *
+ * Description:
+ *   Initialize partitions that are dedicated to firmware OTA update.
+ *
+ * Input Parameters:
+ *   None.
+ *
+ * Returned Value:
+ *   Zero on success; a negated errno value on failure.
+ *
+ ****************************************************************************/
+
+static int init_ota_partitions(void)
+{
+  FAR struct mtd_dev_s *mtd;
+#ifdef CONFIG_BCH
+  char blockdev[18];
+#endif
+  int ret = OK;
+
+  for (uint i = 0; i < ARRAYSIZE(g_ota_partition_table); ++i)
+  {
+      const struct ota_partition_s *part = &g_ota_partition_table[i];
+      mtd = progmem_alloc_mtdpart(part->offset, part->size);
+
+      ret = ftl_initialize(i, mtd);
+      if (ret < 0)
+      {
+          ferr("ERROR: Failed to initialize the FTL layer: %d\n", ret);
+          return ret;
+      }
+
+#ifdef CONFIG_BCH
+      snprintf(blockdev, 18, "/dev/mtdblock%d", i);
+
+      ret = bchdev_register(blockdev, part->devpath, false);
+      if (ret < 0)
+      {
+          ferr("ERROR: bchdev_register %s failed: %d\n", part->devpath, ret);
+          return ret;
+      }
+#endif
+  }
+
+  return ret;
+}
+#endif
+
+
+/****************************************************************************
+ * Public Functions
+ ****************************************************************************/
+
+/****************************************************************************
+ * Name: stm32_progmem_init
+ *
+ * Description:
+ *   Initialize the FLASH and register the MTD device.
+ ****************************************************************************/
+
+int stm32_progmem_init(void)
+{
+  int ret = OK;
+
+
+  stm32h7_flash_unlock();  //TODO: Remove. Is only because cant link if removed.
+
+  /* Create an instance of the SAME70 FLASH program memory device driver */
+
+  g_progmem_mtd = progmem_initialize();
+  if (g_progmem_mtd == NULL)
+  {
+    return -EFAULT;
+  }
+
+#if defined(CONFIG_PROGMEM_OTA_PARTITION)
+  ret = init_ota_partitions();
+  if (ret < 0)
+  {
+    return ret;
+  }
+#else
+  /* Use the FTL layer to wrap the MTD driver as a block driver */
+
+  ret = ftl_initialize(PROGMEM_MTD_MINOR, g_progmem_mtd);
+  if (ret < 0)
+  {
+    syslog(LOG_ERR, "ERROR: Failed to initialize the FTL layer: %d\n",
+          ret);
+    return ret;
+  }
+
+#ifdef CONFIG_BCH
+  char blockdev[18];
+  char chardev[12];
+
+  /* Use the minor number to create device paths */
+
+  snprintf(blockdev, 18, "/dev/mtdblock%d", PROGMEM_MTD_MINOR);
+  snprintf(chardev, 12, "/dev/mtd%d", PROGMEM_MTD_MINOR);
+
+  /* Now create a character device on the block device */
+
+  ret = bchdev_register(blockdev, chardev, false);
+  if (ret < 0)
+  {
+      syslog(LOG_ERR, "ERROR: bchdev_register %s failed: %d\n",
+             chardev, ret);
+      return ret;
+  }
+#endif /* CONFIG_BCH */
+#endif
+
+  return ret;
+}
+
+#endif /* HAVE_PROGMEM_CHARDEV */

Review comment:
       ```suggestion
   #endif /* HAVE_PROGMEM_CHARDEV */
   
   ```




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