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/24 13:03:02 UTC
[GitHub] [incubator-nuttx] pkarashchenko opened a new pull request #4880: MCUboot support for SAM E70 Xplained
pkarashchenko opened a new pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880
## Summary
Initial support of MCUboot for SAMv7 based boards with SAM E70 Xplained as an example
## Impact
SAMv7 board get MCUboot capabilities
## Testing
- Pass CI
- MCUboot loader works on custom SAME70 based 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] Ouss4 commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756086907
##########
File path: boards/arm/samv7/same70-xplained/src/sam_progmem.c
##########
@@ -0,0 +1,267 @@
+/****************************************************************************
+ * boards/arm/samv7/same70-xplained/src/sam_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/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include "sam_progmem.h"
+#include "same70-xplained.h"
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x) (sizeof((x)) / sizeof((x)[0]))
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)
+
+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_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)
+static struct mtd_dev_s *sam_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_samv7_progmem_mtd;
+
+#if defined(CONFIG_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+ {
+ .offset = CONFIG_SAME70XPLAINED_OTA_PRIMARY_SLOT_OFFSET,
+ .size = CONFIG_SAME70XPLAINED_OTA_SLOT_SIZE,
+ .devpath = CONFIG_MCUBOOT_PRIMARY_SLOT_PATH
+ },
+ {
+ .offset = CONFIG_SAME70XPLAINED_OTA_SECONDARY_SLOT_OFFSET,
+ .size = CONFIG_SAME70XPLAINED_OTA_SLOT_SIZE,
+ .devpath = CONFIG_MCUBOOT_SECONDARY_SLOT_PATH
+ },
+ {
+ .offset = CONFIG_SAME70XPLAINED_OTA_SCRATCH_OFFSET,
+ .size = CONFIG_SAME70XPLAINED_OTA_SCRATCH_SIZE,
+ .devpath = CONFIG_MCUBOOT_SCRATCH_PATH
+ }
+};
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)
+/****************************************************************************
+ * Name: esp32_sam_progmem_alloc_mtdpart
Review comment:
```suggestion
#if defined(CONFIG_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)
/****************************************************************************
* Name: sam_progmem_alloc_mtdpart
```
##########
File path: boards/arm/samv7/same70-xplained/src/sam_progmem.c
##########
@@ -0,0 +1,267 @@
+/****************************************************************************
+ * boards/arm/samv7/same70-xplained/src/sam_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/kmalloc.h>
+#include <nuttx/mtd/mtd.h>
+#ifdef CONFIG_BCH
+#include <nuttx/drivers/drivers.h>
+#endif
+
+#include "sam_progmem.h"
+#include "same70-xplained.h"
+
+#ifdef HAVE_PROGMEM_CHARDEV
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#define ARRAYSIZE(x) (sizeof((x)) / sizeof((x)[0]))
+
+/****************************************************************************
+ * Private Types
+ ****************************************************************************/
+
+#if defined(CONFIG_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)
+
+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_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)
+static struct mtd_dev_s *sam_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_samv7_progmem_mtd;
+
+#if defined(CONFIG_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)
+static const struct ota_partition_s g_ota_partition_table[] =
+{
+ {
+ .offset = CONFIG_SAME70XPLAINED_OTA_PRIMARY_SLOT_OFFSET,
+ .size = CONFIG_SAME70XPLAINED_OTA_SLOT_SIZE,
+ .devpath = CONFIG_MCUBOOT_PRIMARY_SLOT_PATH
+ },
+ {
+ .offset = CONFIG_SAME70XPLAINED_OTA_SECONDARY_SLOT_OFFSET,
+ .size = CONFIG_SAME70XPLAINED_OTA_SLOT_SIZE,
+ .devpath = CONFIG_MCUBOOT_SECONDARY_SLOT_PATH
+ },
+ {
+ .offset = CONFIG_SAME70XPLAINED_OTA_SCRATCH_OFFSET,
+ .size = CONFIG_SAME70XPLAINED_OTA_SCRATCH_SIZE,
+ .devpath = CONFIG_MCUBOOT_SCRATCH_PATH
+ }
+};
+#endif
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+#if defined(CONFIG_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)
+/****************************************************************************
+ * Name: esp32_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:
+ * ESP32 SPI Flash MTD data pointer if success or NULL if fail
Review comment:
```suggestion
* MTD partition data pointer on success, NULL on failure.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756372893
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
+ depends on BOOT_MCUBOOT
+ ---help---
+ The SAMV7 port of MCUboot supports the loading of unsegmented firmware
+ images.
Review comment:
> [...] , but again I think that "segmented" firmware is not ESP32 specific, but more related to code located in multiple memory regions that is usually a use case when code is executed from RAM.
That's what I tried to explain in my previous answer, but I might not have made myself clear.
**Segments** per se are not exclusive to ESP32. Memory segments are specified in the ELF file format.
But that specific "unsegmented" term on the help text should refer to a characteristic of the Espressif file format, and that probably does not make sense to state in this context because that's no different than the non-MCUboot behavior for this chip.
For a ESP32 "segmented image" the information for each segment is stored in the Image Header, which allows the segments to be juxtaposed in the binary file, resulting in a denser binary file.
The "unsegmented" term should mean that the MCUboot image format for ESP32 does not follow the same approach. Instead it relies on the VMA and LMA information provided via linker file, similar to the linker scripts from other chip vendors.
--
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 #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756106976
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
Review comment:
In general when "MCUBOOT_BOOTLOADER" is enabled then it needs access to OTA partitions.
I think that my idea was to use `#if defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)` only instead of `#if defined(CONFIG_MCUBOOT_BOOTLOADER) || defined(CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT)`, but seems that I didn't finalized it.
I will double check and come back with some comments
--
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 #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756104620
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
+ depends on BOOT_MCUBOOT
+ ---help---
+ The SAMV7 port of MCUboot supports the loading of unsegmented firmware
+ images.
Review comment:
I think that segmented firmware image is not unique to ESP32. In MCUboot terminology it is called "Multiple image boot". I mean that if firmware is segmented it can be loaded using "Multiple image boot", but not that those two terms are equal. I can update text to avoid "unsegmented" term if needed.
--
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 #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756352097
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
Review comment:
Can I still use `depends on BOOT_MCUBOOT` in OS repository? Or those should be completely separated?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei merged pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
gustavonihei merged pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880
--
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 #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#issuecomment-977886929
@pkarashchenko there is a minor style warning:
https://github.com/apache/incubator-nuttx/runs/4312524794?check_suite_focus=true
Please fix 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 pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#issuecomment-978840334
> @pkarashchenko there is a minor style warning: https://github.com/apache/incubator-nuttx/runs/4312524794?check_suite_focus=true Please fix it, thanks.
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] gustavonihei commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756334574
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
+ depends on BOOT_MCUBOOT
+ ---help---
+ The SAMV7 port of MCUboot supports the loading of unsegmented firmware
+ images.
Review comment:
"Multiple image boot" is unrelated to the segmentation we are discussing here. The **segments** relate to the data and code sections from a given executable firmware image. Think about the sections of an ELF file.
The ESP32 image format organizes these sections (segments) differently, optimized for the chip's memory organization: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/system/app_image_format.html
So, although it might not be unique to ESP32, that's the segmentation I was referring to.
The MCUboot's "Multiple image boot" is literally support for multiple firmware images, but **per image slot**. So this enables the device to support the execution of a different application firmware image, e.g. a recovery firmware image as the second image on the slot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] Ouss4 commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
Ouss4 commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756088974
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
Review comment:
Is this correct?
--
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 #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756344373
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
+ depends on BOOT_MCUBOOT
+ ---help---
+ The SAMV7 port of MCUboot supports the loading of unsegmented firmware
+ images.
Review comment:
Agree. Thank for the clarification. Now I see that "Multiple image boot" does not handle segmented load, but again I think that "segmented" firmware is not ESP32 specific, but more related to code located in multiple memory regions that is usually a use case when code is executed from RAM. I see that MCUboot "RAM loading" support the RAM load for a single segment of code and in case if we need a multiple segment load the additional development should be done on MCUboot side.
--
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 #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#issuecomment-979034211
Currently build fails because of `-Werror` is used for PreCI builds. The fix is submitted to MCUboot and waiting to be merged: https://github.com/mcu-tools/mcuboot/pull/1222
--
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 #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756346766
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
Review comment:
Ok. I will redo and separate those parts. `CONFIG_SAME70XPLAINED_APP_FORMAT_MCUBOOT` will only be an option for linker file selection and probably I will add additional option like `CONFIG_SAME70XPLAINED_HAVE_OTA_PARTITION`.
But then comes a question if I can still use `#if defined(CONFIG_MCUBOOT_BOOTLOADER)` in board specific files because it is an application level config?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756570126
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
+ depends on BOOT_MCUBOOT
+ ---help---
+ The SAMV7 port of MCUboot supports the loading of unsegmented firmware
+ images.
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] gustavonihei commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756089582
##########
File path: arch/arm/src/samv7/Kconfig
##########
@@ -816,6 +816,13 @@ config SAMV7_PROGMEM_NSECTORS
flash memory that will be reserved for use with by the interfaces
prototyped in include/nuttx/progmem.h
+config SAMV7_PROGMEM_ERASESTATE
+ bool "Flash progmem erasestate ictl support"
Review comment:
```suggestion
bool "Flash progmem erasestate ioctl support"
```
##########
File path: arch/arm/src/samv7/Kconfig
##########
@@ -816,6 +816,13 @@ config SAMV7_PROGMEM_NSECTORS
flash memory that will be reserved for use with by the interfaces
prototyped in include/nuttx/progmem.h
+config SAMV7_PROGMEM_ERASESTATE
+ bool "Flash progmem erasestate ictl support"
+ default y
+ select ARCH_HAVE_PROGMEM_ERASESTATE
+ ---help---
+ Add progmem erasestate ictl command.
Review comment:
```suggestion
Add progmem erasestate ioctl command.
```
--
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 #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#issuecomment-977901874
> @pkarashchenko there is a minor style warning: https://github.com/apache/incubator-nuttx/runs/4312524794?check_suite_focus=true Please fix it, thanks.
I'm not sure what is wrong here and how to fix it. `((void (*)(void))vt.reset)();` seems to be fine to me. Could you please advice how code should look like?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756372893
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
+ depends on BOOT_MCUBOOT
+ ---help---
+ The SAMV7 port of MCUboot supports the loading of unsegmented firmware
+ images.
Review comment:
> [...] , but again I think that "segmented" firmware is not ESP32 specific, but more related to code located in multiple memory regions that is usually a use case when code is executed from RAM.
That's what I tried to explain in my previous answer, but I might not have made myself clear.
**Segments** per se are not exclusive to ESP32. Memory segments are specified in the ELF file format.
But that specific "unsegmented" term on the help text should refer to a characteristic of the Espressif file format (or lack of it), and that probably does not make sense to state in this context because that's no different than the non-MCUboot behavior for this chip.
For a ESP32 "segmented image" the information for each segment is stored in the Image Header, which allows the segments to be juxtaposed in the binary file, resulting in a denser binary file.
The "unsegmented" term should mean that the MCUboot image format for ESP32 does not follow the same approach. Instead it relies on the VMA and LMA information provided via linker file, similar to the linker scripts from other chip vendors.
--
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 #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756570217
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
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] gustavonihei commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756334574
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
+ depends on BOOT_MCUBOOT
+ ---help---
+ The SAMV7 port of MCUboot supports the loading of unsegmented firmware
+ images.
Review comment:
"Multiple image boot" is unrelated to the segmentation we are discussing here. The **segments** relate to the data and code sections from a given executable firmware image. Think about the output sections of an ELF file (but not exactly that).
The ESP32 image format organizes these sections (segments) differently, optimized for the chip's memory organization: https://docs.espressif.com/projects/esp-idf/en/latest/esp32/api-reference/system/app_image_format.html
So, although it might not be unique to ESP32, that's the segmentation I was referring to.
The MCUboot's "Multiple image boot" is literally support for multiple firmware images, but **per image slot**. So this enables the device to support the execution of a different application firmware image, e.g. a recovery firmware image as the second image on the slot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756342658
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
Review comment:
`MCUBOOT_BOOTLOADER` being a config from the APPS repository should not be referenced on the OS repository.
Conceptually speaking, the generation of the NuttX binary in MCUboot format does not depend on the MCUboot library or application being integrated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756095708
##########
File path: boards/arm/samv7/same70-xplained/src/sam_boot_image.c
##########
@@ -0,0 +1,156 @@
+/****************************************************************************
+ * boards/arm/samv7/same70-xplained/src/sam_boot_image.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);
+
+/****************************************************************************
+ * 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 < SAM_IRQ_NIRQS; i += 32)
+ {
+ putreg32(0xffffffff, NVIC_IRQ_CLEAR(i));
+ }
+
+ /* Clear all pending interrupts */
+
+ for (i = 0; i < SAM_IRQ_NIRQS; i += 32)
+ {
+ putreg32(0xffffffff, NVIC_IRQ_CLRPEND(i));
+ }
+}
+
+/****************************************************************************
+ * 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 vecore table: %d", bytes);
Review comment:
```suggestion
syslog(LOG_ERR, "Failed to read ARM vector table: %d", bytes);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] gustavonihei commented on a change in pull request #4880: MCUboot support for SAM E70 Xplained
Posted by GitBox <gi...@apache.org>.
gustavonihei commented on a change in pull request #4880:
URL: https://github.com/apache/incubator-nuttx/pull/4880#discussion_r756092352
##########
File path: boards/arm/samv7/same70-xplained/Kconfig
##########
@@ -90,4 +90,38 @@ config SAME70XPLAINED_HSMCI0_AUTOMOUNT_UDELAY
endif # SAME70XPLAINED_HSMCI0_AUTOMOUNT
+menuconfig SAME70XPLAINED_APP_FORMAT_MCUBOOT
+ bool "MCUboot-bootable format"
+ default n || MCUBOOT_BOOTLOADER
+ depends on BOOT_MCUBOOT
+ ---help---
+ The SAMV7 port of MCUboot supports the loading of unsegmented firmware
+ images.
Review comment:
It seems this help text was based on the one from ESP32. The standard Image Format for ESP32 defines image segments, which differs from the MCUboot image format (that explains the "unsegmented" term).
You may want to rewrite this help text to adequate to this different context.
--
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